SDL crash during cleanup. #2621
Conversation
If resumption_storage_ in ResumeCtrlImpl is failed to initialize, SDL will try to stop components and exit with error code. At this time application manager tries to unregister all applications and call ResumeCtrlImpl::OnIgnitionOff() where resumption_storage_ is used. As a result, a crash will occur. As an example, if AppStorageFolder have no read/write access, resumption_storage_ fails to initialize.
mkorniichuk
requested review from
AKalinich-Luxoft,
Itileda,
LuxoftAKutsan,
LitvinenkoIra,
Ypostolov,
BSolonenko,
a user,
APipko,
ZhdanovP and
mked-luxoft
| @@ -336,6 +336,10 @@ void ResumeCtrlImpl::OnSuspend() { | |||
| void ResumeCtrlImpl::OnIgnitionOff() { | |||
| LOG4CXX_AUTO_TRACE(logger_); | |||
| if (!application_manager_.IsLowVoltage()) { | |||
| if (!resumption_storage_) { | |||
There was a problem hiding this comment.
@mkorniichuk do we have case if resumption storage may be nit initialized? If no> I would put DCHECK here
There was a problem hiding this comment.
@mkorniichuk in case if FS is read only, it should be handled inside resumption storage, isn't it? Or even in Last state.
There was a problem hiding this comment.
@LuxoftAKutsan DCHECK will stop application immediately with logged fatal error. But we still need to stop all components, don't we?
About the FS. It is handled in application manager Init(). So the Init() of resumption_storage is not even called. Then StopComponents takes place, where Stop method is called for application manager, and so the OnIgnitonOff method of ResumptionController is called as well. And since resumption_storage is not initialized, a crash will occur.
There was a problem hiding this comment.
DCHECK is assertion. Assertion will be removed from release build. During testing if we found such condition, we can just crash SDL with appropriate information.
src/components/application_manager/src/resumption/resume_ctrl_impl.cc
Outdated
Show resolved
Hide resolved
| if (!resumption_storage_) { | ||
| LOG4CXX_ERROR(logger_, "resumption_storage_ is not initialized"); | ||
| return; | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
src/components/application_manager/src/resumption/resume_ctrl_impl.cc
Outdated
Show resolved
Hide resolved
jacobkeeler
deleted the
fix/SDL_crash_during_cleanup_if_storage_not_initialized
branch
* SDL crash during StopComponents() if StartComponents() failed. If resumption_storage_ in ResumeCtrlImpl is failed to initialize, SDL will try to stop components and exit with error code. At this time application manager tries to unregister all applications and call ResumeCtrlImpl::OnIgnitionOff() where resumption_storage_ is used. As a result, a crash will occur. As an example, if AppStorageFolder have no read/write access, resumption_storage_ fails to initialize. Co-authored-by: Jacob Keeler <jacob.keeler@livioradio.com>
Fixes #2428 #1829
This PR is ready for review.
Risk
This PR makes no API changes.
Testing Plan
Manually tested
Summary
If resumption_storage_ in ResumeCtrlImpl is failed to initialize,
SDL will try to stop components and exit with error code.
At this time application manager tries to unregister all applications and
call ResumeCtrlImpl::OnIgnitionOff() where resumption_storage_ is used. As a result, a crash will occur.
As an example, if AppStorageFolder have no read/write access, resumption_storage_
fails to initialize.
CLA