diff options
author | Igor Gapchuk (GitHub) <41586842+IGapchuk@users.noreply.github.com> | 2019-09-12 18:03:05 +0300 |
---|---|---|
committer | Collin <iCollin@users.noreply.github.com> | 2019-09-12 11:03:05 -0400 |
commit | 2449d7e0e71db6cc07f50fd427a4a5a7dd037015 (patch) | |
tree | 0492ab921951559a6e029fc9dc855e2d501698e9 | |
parent | 5b9e0c0ade33c1eb1a30de14fdc64f5efeaf9cc8 (diff) | |
download | sdl_core-2449d7e0e71db6cc07f50fd427a4a5a7dd037015.tar.gz |
Fix SDL's core crash during retry sequence (#3020)
* Fix SDL crash during retry sequence
In case, when lust application is unregistered, SDL has to stop retry
sequence, because doesn't exist any mobile connection and these actions are
redundant.
Also SDL should check if exist registered applications before sending
PolicySnapshotNotification.
* fixup! Fix SDL crash during retry sequence
* fixup! Fix SDL crash during retry sequence
14 files changed, 79 insertions, 9 deletions
diff --git a/src/components/application_manager/include/application_manager/policies/policy_handler.h b/src/components/application_manager/include/application_manager/policies/policy_handler.h index 141c679aad..9ca7761426 100644 --- a/src/components/application_manager/include/application_manager/policies/policy_handler.h +++ b/src/components/application_manager/include/application_manager/policies/policy_handler.h @@ -685,6 +685,8 @@ class PolicyHandler : public PolicyHandlerInterface, virtual void OnPTInited() OVERRIDE; + void StopRetrySequence() OVERRIDE; + /** * @brief OnDeviceSwitching Notifies policy manager on device switch event so * policy permissions should be processed accordingly diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc index 3d9aec65c2..46ccb86e8b 100644 --- a/src/components/application_manager/src/application_manager_impl.cc +++ b/src/components/application_manager/src/application_manager_impl.cc @@ -3092,6 +3092,9 @@ void ApplicationManagerImpl::UnregisterApplication( MessageHelper::SendOnAppUnregNotificationToHMI( app_to_remove, is_unexpected_disconnect, *this); request_ctrl_.terminateAppRequests(app_id); + if (applications_.empty()) { + policy_handler_->StopRetrySequence(); + } return; } diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index ac428426e6..f88eaf47a5 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -391,6 +391,12 @@ void PolicyHandler::OnPTInited() { std::mem_fun(&PolicyHandlerObserver::OnPTInited)); } +void PolicyHandler::StopRetrySequence() { + LOG4CXX_AUTO_TRACE(logger_); + + policy_manager_->StopRetrySequence(); +} + bool PolicyHandler::ResetPolicyTable() { LOG4CXX_TRACE(logger_, "Reset policy table."); POLICY_LIB_CHECK(false); @@ -442,6 +448,11 @@ uint32_t PolicyHandler::GetAppIdForSending() const { logger_, "Number of apps with NONE level: " << apps_with_none_level.size()); + if (apps_with_none_level.empty()) { + LOG4CXX_WARN(logger_, "There is no registered application"); + return 0; + } + return ChooseRandomAppForPolicyUpdate(apps_with_none_level); } @@ -1522,8 +1533,13 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string, } if (PTUIterationType::RetryIteration == iteration_type) { - MessageHelper::SendPolicySnapshotNotification( - GetAppIdForSending(), pt_string, std::string(""), application_manager_); + uint32_t app_id_for_sending = GetAppIdForSending(); + + if (0 != app_id_for_sending) { + MessageHelper::SendPolicySnapshotNotification( + app_id_for_sending, pt_string, std::string(), application_manager_); + } + } else { MessageHelper::SendPolicyUpdate( policy_snapshot_full_path, diff --git a/src/components/application_manager/test/application_manager_impl_test.cc b/src/components/application_manager/test/application_manager_impl_test.cc index bb87d71a48..bfd815cd8b 100644 --- a/src/components/application_manager/test/application_manager_impl_test.cc +++ b/src/components/application_manager/test/application_manager_impl_test.cc @@ -88,6 +88,8 @@ using ::testing::SaveArg; using ::testing::SetArgPointee; using ::testing::SetArgReferee; +using test::components::policy_test::MockPolicyHandlerInterface; + using namespace application_manager; // custom action to call a member function with 4 arguments @@ -146,12 +148,14 @@ class ApplicationManagerImplTest std::make_shared<NiceMock<resumption_test::MockResumptionData> >( mock_app_mngr_)) , mock_rpc_service_(new MockRPCService) - , mock_policy_handler_( - new test::components::policy_test::MockPolicyHandlerInterface) + , mock_policy_handler_(new NiceMock<MockPolicyHandlerInterface>) , mock_app_service_manager_( new MockAppServiceManager(mock_app_mngr_, mock_last_state_)) , mock_message_helper_( application_manager::MockMessageHelper::message_helper_mock()) + , mock_statistics_manager_( + std::make_shared< + NiceMock<usage_statistics_test::MockStatisticsManager> >()) { #ifdef ENABLE_LOG @@ -159,7 +163,7 @@ class ApplicationManagerImplTest #endif Mock::VerifyAndClearExpectations(mock_message_helper_); } - ~ApplicationManagerImplTest() { + ~ApplicationManagerImplTest() OVERRIDE { Mock::VerifyAndClearExpectations(mock_message_helper_); } @@ -186,6 +190,13 @@ class ApplicationManagerImplTest app_manager_impl_->SetAppServiceManager(mock_app_service_manager_); Json::Value empty; ON_CALL(mock_last_state_, get_dictionary()).WillByDefault(ReturnRef(empty)); + + auto request = std::make_shared<smart_objects::SmartObject>( + smart_objects::SmartType_Map); + ON_CALL(*mock_message_helper_, CreateModuleInfoSO(_, _)) + .WillByDefault(Return(request)); + ON_CALL(*mock_policy_handler_, GetStatisticManager()) + .WillByDefault(Return(mock_statistics_manager_)); } void CreateAppManager() { @@ -212,6 +223,7 @@ class ApplicationManagerImplTest mock_application_manager_settings_, mock_policy_settings_)); mock_app_ptr_ = std::shared_ptr<MockApplication>(new MockApplication()); app_manager_impl_->set_protocol_handler(&mock_protocol_handler_); + app_manager_impl_->SetMockPolicyHandler(mock_policy_handler_); ASSERT_TRUE(app_manager_impl_.get()); ASSERT_TRUE(mock_app_ptr_.get()); } @@ -266,8 +278,7 @@ class ApplicationManagerImplTest NiceMock<con_test::MockConnectionHandler> mock_connection_handler_; NiceMock<protocol_handler_test::MockSessionObserver> mock_session_observer_; NiceMock<MockApplicationManagerSettings> mock_application_manager_settings_; - test::components::policy_test::MockPolicyHandlerInterface* - mock_policy_handler_; + NiceMock<MockPolicyHandlerInterface>* mock_policy_handler_; application_manager_test::MockApplicationManager mock_app_mngr_; std::unique_ptr<am::ApplicationManagerImpl> app_manager_impl_; MockAppServiceManager* mock_app_service_manager_; @@ -275,6 +286,8 @@ class ApplicationManagerImplTest std::shared_ptr<MockApplication> mock_app_ptr_; NiceMock<protocol_handler_test::MockProtocolHandler> mock_protocol_handler_; + std::shared_ptr<NiceMock<usage_statistics_test::MockStatisticsManager> > + mock_statistics_manager_; }; INSTANTIATE_TEST_CASE_P( @@ -1558,7 +1571,6 @@ TEST_F(ApplicationManagerImplTest, #if defined(CLOUD_APP_WEBSOCKET_TRANSPORT_SUPPORT) void ApplicationManagerImplTest::AddCloudAppToPendingDeviceMap() { - app_manager_impl_->SetMockPolicyHandler(mock_policy_handler_); std::vector<std::string> enabled_apps{"1234"}; EXPECT_CALL(*mock_policy_handler_, GetEnabledCloudApps(_)) .WillOnce(SetArgReferee<0>(enabled_apps)); @@ -1775,7 +1787,6 @@ TEST_F(ApplicationManagerImplTest, } TEST_F(ApplicationManagerImplTest, PolicyIDByIconUrl_Success) { - app_manager_impl_->SetMockPolicyHandler(mock_policy_handler_); std::vector<std::string> enabled_apps{"1234"}; EXPECT_CALL(*mock_policy_handler_, GetEnabledCloudApps(_)) .WillOnce(SetArgReferee<0>(enabled_apps)); diff --git a/src/components/include/application_manager/policies/policy_handler_interface.h b/src/components/include/application_manager/policies/policy_handler_interface.h index f6ecd050ae..190a1f786b 100644 --- a/src/components/include/application_manager/policies/policy_handler_interface.h +++ b/src/components/include/application_manager/policies/policy_handler_interface.h @@ -351,6 +351,11 @@ class PolicyHandlerInterface : public VehicleDataItemProvider { virtual void OnPTInited() = 0; + /** + * @brief Force stops retry sequence timer and resets retry sequence + */ + virtual void StopRetrySequence() = 0; + #ifdef EXTERNAL_PROPRIETARY_MODE virtual void OnCertificateDecrypted(bool is_succeeded) = 0; #endif // EXTERNAL_PROPRIETARY_MODE diff --git a/src/components/include/policy/policy_external/policy/policy_manager.h b/src/components/include/policy/policy_external/policy/policy_manager.h index cce52da626..4d5a2000b6 100644 --- a/src/components/include/policy/policy_external/policy/policy_manager.h +++ b/src/components/include/policy/policy_external/policy/policy_manager.h @@ -201,6 +201,11 @@ class PolicyManager : public usage_statistics::StatisticsManager, virtual std::string ForcePTExchange() = 0; /** + * @brief Stops retry sequence timer and resets retry sequence + */ + virtual void StopRetrySequence() = 0; + + /** * @brief Exchange by user request * @return Current status of policy table */ diff --git a/src/components/include/policy/policy_regular/policy/policy_manager.h b/src/components/include/policy/policy_regular/policy/policy_manager.h index def0a3660e..37a3fdd01c 100644 --- a/src/components/include/policy/policy_regular/policy/policy_manager.h +++ b/src/components/include/policy/policy_regular/policy/policy_manager.h @@ -202,6 +202,11 @@ class PolicyManager : public usage_statistics::StatisticsManager, virtual std::string ForcePTExchangeAtUserRequest() = 0; /** + * @brief Stops retry sequence timer and resets retry sequence + */ + virtual void StopRetrySequence() = 0; + + /** * @brief Resets retry sequence * @param send_event - if true corresponding event is sent to * UpdateStatusManager diff --git a/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h b/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h index 3b6a4af54d..deb405cbde 100644 --- a/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h +++ b/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h @@ -194,6 +194,7 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface { MOCK_METHOD1(OnCertificateUpdated, void(const std::string& certificate_data)); MOCK_METHOD1(OnPTUFinished, void(const bool ptu_result)); MOCK_METHOD0(OnPTInited, void()); + MOCK_METHOD0(StopRetrySequence, void()); MOCK_METHOD1(OnCertificateDecrypted, void(bool is_succeeded)); MOCK_METHOD0(CanUpdate, bool()); MOCK_METHOD2(OnDeviceConsentChanged, diff --git a/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h b/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h index 455954795f..c1d4c5c1c8 100644 --- a/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h +++ b/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h @@ -102,6 +102,7 @@ class MockPolicyManager : public PolicyManager { MOCK_METHOD0(IncrementIgnitionCycles, void()); MOCK_METHOD0(ForcePTExchange, std::string()); MOCK_METHOD0(ForcePTExchangeAtUserRequest, std::string()); + MOCK_METHOD0(StopRetrySequence, void()); MOCK_METHOD1(ResetRetrySequence, void(const policy::ResetRetryCountType send_event)); MOCK_METHOD0(NextRetryTimeout, int()); diff --git a/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h b/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h index a446128cad..9ed8571804 100644 --- a/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h +++ b/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h @@ -103,6 +103,7 @@ class MockPolicyManager : public PolicyManager { MOCK_METHOD0(IncrementIgnitionCycles, void()); MOCK_METHOD0(ForcePTExchange, std::string()); MOCK_METHOD0(ForcePTExchangeAtUserRequest, std::string()); + MOCK_METHOD0(StopRetrySequence, void()); MOCK_METHOD1(ResetRetrySequence, void(const policy::ResetRetryCountType send_event)); MOCK_METHOD0(NextRetryTimeout, uint32_t()); diff --git a/src/components/policy/policy_external/include/policy/policy_manager_impl.h b/src/components/policy/policy_external/include/policy/policy_manager_impl.h index 497394886f..321294aa3d 100644 --- a/src/components/policy/policy_external/include/policy/policy_manager_impl.h +++ b/src/components/policy/policy_external/include/policy/policy_manager_impl.h @@ -207,6 +207,8 @@ class PolicyManagerImpl : public PolicyManager { */ std::string ForcePTExchange() OVERRIDE; + void StopRetrySequence() OVERRIDE; + /** * @brief Exchange by user request * @return Current status of policy table diff --git a/src/components/policy/policy_external/src/policy_manager_impl.cc b/src/components/policy/policy_external/src/policy_manager_impl.cc index 9b9d300d78..95d3011b33 100644 --- a/src/components/policy/policy_external/src/policy_manager_impl.cc +++ b/src/components/policy/policy_external/src/policy_manager_impl.cc @@ -1877,6 +1877,12 @@ std::string PolicyManagerImpl::ForcePTExchange() { return update_status_manager_.StringifiedUpdateStatus(); } +void policy::PolicyManagerImpl::StopRetrySequence() { + LOG4CXX_AUTO_TRACE(logger_); + + ResetRetrySequence(ResetRetryCountType::kResetWithStatusUpdate); +} + std::string PolicyManagerImpl::ForcePTExchangeAtUserRequest() { LOG4CXX_AUTO_TRACE(logger_); update_status_manager_.ScheduleManualUpdate(); diff --git a/src/components/policy/policy_regular/include/policy/policy_manager_impl.h b/src/components/policy/policy_regular/include/policy/policy_manager_impl.h index 0bc311ba9f..542f33794b 100644 --- a/src/components/policy/policy_regular/include/policy/policy_manager_impl.h +++ b/src/components/policy/policy_regular/include/policy/policy_manager_impl.h @@ -237,6 +237,8 @@ class PolicyManagerImpl : public PolicyManager { */ std::string ForcePTExchange() OVERRIDE; + void StopRetrySequence() OVERRIDE; + /** * @brief Exchange by user request * @return Current status of policy table diff --git a/src/components/policy/policy_regular/src/policy_manager_impl.cc b/src/components/policy/policy_regular/src/policy_manager_impl.cc index 0e1cfd267a..9e5cf01493 100644 --- a/src/components/policy/policy_regular/src/policy_manager_impl.cc +++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc @@ -1229,6 +1229,16 @@ std::string PolicyManagerImpl::ForcePTExchange() { return update_status_manager_.StringifiedUpdateStatus(); } +void PolicyManagerImpl::StopRetrySequence() { + LOG4CXX_AUTO_TRACE(logger_); + + if (timer_retry_sequence_.is_running()) { + timer_retry_sequence_.Stop(); + } + + ResetRetrySequence(ResetRetryCountType::kResetWithStatusUpdate); +} + std::string PolicyManagerImpl::ForcePTExchangeAtUserRequest() { update_status_manager_.ScheduleManualUpdate(); StartPTExchange(); |