From 87f380084b03858dcb63630fa650002d8c9fdb21 Mon Sep 17 00:00:00 2001 From: sniukalov Date: Fri, 9 Nov 2018 13:34:51 +0200 Subject: [PATCH 1/4] Fixed vehicle data after master reset. --- .../policies/policy_handler.h | 2 +- .../src/policies/policy_handler.cc | 231 +++++++++++++----- 2 files changed, 173 insertions(+), 60 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 c8f3bcf888f..4e544e0687c 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 @@ -731,7 +731,7 @@ class PolicyHandler : public PolicyHandlerInterface, void GetRegisteredLinks(std::map& out_links) const; private: - mutable sync_primitives::RWLock policy_manager_lock_; + mutable sync_primitives::RecursiveLock policy_manager_lock_; std::shared_ptr policy_manager_; void* dl_handle_; std::shared_ptr event_observer_; diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 1b64c5d7b4b..a5cd3ee14a4 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -136,7 +136,7 @@ struct HMILevelPredicate #define POLICY_LIB_CHECK(return_value) \ { \ - sync_primitives::AutoReadLock lock(policy_manager_lock_); \ + LOG4CXX_AUTO_TRACE(logger_); \ if (!policy_manager_) { \ LOG4CXX_DEBUG(logger_, "The shared library of policy is not loaded"); \ return return_value; \ @@ -145,7 +145,7 @@ struct HMILevelPredicate #define POLICY_LIB_CHECK_VOID() \ { \ - sync_primitives::AutoReadLock lock(policy_manager_lock_); \ + LOG4CXX_AUTO_TRACE(logger_); \ if (!policy_manager_) { \ LOG4CXX_DEBUG(logger_, "The shared library of policy is not loaded"); \ return; \ @@ -179,9 +179,11 @@ struct DeactivateApplication { struct SDLAllowedNotification { SDLAllowedNotification(const connection_handler::DeviceHandle& device_id, PolicyManager* policy_manager, + sync_primitives::RecursiveLock& policy_manager_lock, StateController& state_controller) : device_id_(device_id) , policy_manager_(policy_manager) + , policy_manager_lock_(policy_manager_lock) , state_controller_(state_controller) {} void operator()(const ApplicationSharedPtr& app) { @@ -189,7 +191,10 @@ struct SDLAllowedNotification { if (device_id_ == app->device()) { std::string hmi_level = "NONE"; mobile_apis::HMILevel::eType default_mobile_hmi; - policy_manager_->GetDefaultHmi(app->policy_app_id(), &hmi_level); + { + sync_primitives::AutoLock lock(policy_manager_lock_); + policy_manager_->GetDefaultHmi(app->policy_app_id(), &hmi_level); + } if ("BACKGROUND" == hmi_level) { default_mobile_hmi = mobile_apis::HMILevel::HMI_BACKGROUND; } else if ("FULL" == hmi_level) { @@ -208,6 +213,7 @@ struct SDLAllowedNotification { private: connection_handler::DeviceHandle device_id_; PolicyManager* policy_manager_; + sync_primitives::RecursiveLock& policy_manager_lock_; StateController& state_controller_; }; @@ -339,11 +345,11 @@ bool PolicyHandler::PolicyEnabled() const { bool PolicyHandler::LoadPolicyLibrary() { LOG4CXX_AUTO_TRACE(logger_); - sync_primitives::AutoWriteLock lock(policy_manager_lock_); if (!PolicyEnabled()) { LOG4CXX_WARN(logger_, "System is configured to work without policy " "functionality."); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_.reset(); return false; } @@ -352,7 +358,10 @@ bool PolicyHandler::LoadPolicyLibrary() { const char* error = dlerror(); if (!error) { if (CreateManager()) { + policy_manager_lock_.Acquire(); policy_manager_->set_listener(this); + policy_manager_lock_.Release(); + event_observer_ = std::shared_ptr(new PolicyEventObserver( this, application_manager_.event_dispatcher())); @@ -386,7 +395,6 @@ const PolicySettings& PolicyHandler::get_settings() const { } bool PolicyHandler::InitPolicyTable() { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK(false); // Subscribing to notification for system readiness to be able to get system // info necessary for policy table @@ -394,6 +402,7 @@ bool PolicyHandler::InitPolicyTable() { hmi_apis::FunctionID::BasicCommunication_OnReady); std::string preloaded_file = get_settings().preloaded_pt_file(); if (file_system::FileExists(preloaded_file)) { + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->InitPT(preloaded_file, &get_settings()); } LOG4CXX_FATAL(logger_, "The file which contains preloaded PT is not exist"); @@ -401,10 +410,10 @@ bool PolicyHandler::InitPolicyTable() { } bool PolicyHandler::ResetPolicyTable() { - LOG4CXX_TRACE(logger_, "Reset policy table."); POLICY_LIB_CHECK(false); std::string preloaded_file = get_settings().preloaded_pt_file(); if (file_system::FileExists(preloaded_file)) { + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->ResetPT(preloaded_file); } LOG4CXX_WARN(logger_, "The file which contains preloaded PT is not exist"); @@ -412,13 +421,12 @@ bool PolicyHandler::ResetPolicyTable() { } bool PolicyHandler::ClearUserConsent() { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK(false); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->ResetUserConsent(); } uint32_t PolicyHandler::GetAppIdForSending() const { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK(0); const ApplicationSet& accessor = application_manager_.applications().GetData(); @@ -487,6 +495,8 @@ void PolicyHandler::OnDeviceConsentChanged(const std::string& device_id, // back to their own permissions, if device allowed again, and must be // notified about these changes + sync_primitives::AutoLock lock(policy_manager_lock_); + const ApplicationSet& accessor = application_manager_.applications().GetData(); ApplicationSetConstIt it_app_list = accessor.begin(); @@ -528,8 +538,8 @@ void PolicyHandler::SendOnAppPermissionsChanged( } void PolicyHandler::OnPTExchangeNeeded() { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->ForcePTExchange(); } @@ -555,20 +565,21 @@ StatusNotifier PolicyHandler::AddApplication( const std::string& application_id, const rpc::policy_table_interface_base::AppHmiTypes& hmi_types) { POLICY_LIB_CHECK(std::make_shared()); + sync_primitives::AutoLock lock_policy(policy_manager_lock_); return policy_manager_->AddApplication(application_id, hmi_types); } void PolicyHandler::AddDevice(const std::string& device_id, const std::string& connection_type) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->AddDevice(device_id, connection_type); } void PolicyHandler::SetDeviceInfo(const std::string& device_id, const DeviceInfo& device_info) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->SetDeviceInfo(device_id, device_info); } @@ -577,7 +588,6 @@ void PolicyHandler::OnAppPermissionConsentInternal( const uint32_t connection_key, const ExternalConsentStatus& external_consent_status, PermissionConsent& out_permissions) { - LOG4CXX_AUTO_TRACE(logger_); const PolicyManager::NotificationMode mode = external_consent_status.empty() ? PolicyManager::kNotifyApplicationMode : PolicyManager::kSilentMode; @@ -600,6 +610,7 @@ void PolicyHandler::OnAppPermissionConsentInternal( } if (!out_permissions.policy_app_id.empty()) { + sync_primitives::AutoLock lock(policy_manager_lock_); #ifdef EXTERNAL_PROPRIETARY_MODE policy_manager_->SetUserConsentForApp(out_permissions, mode); #else @@ -637,11 +648,14 @@ void PolicyHandler::OnAppPermissionConsentInternal( out_permissions.policy_app_id = it->second; out_permissions.device_id = it->first; + + policy_manager_lock_.Acquire(); #ifdef EXTERNAL_PROPRIETARY_MODE policy_manager_->SetUserConsentForApp(out_permissions, mode); #else policy_manager_->SetUserConsentForApp(out_permissions); #endif + policy_manager_lock_.Release(); } } else { LOG4CXX_WARN(logger_, @@ -649,6 +663,7 @@ void PolicyHandler::OnAppPermissionConsentInternal( "setting common permissions."); } #ifdef EXTERNAL_PROPRIETARY_MODE + sync_primitives::AutoLock lock(policy_manager_lock_); if (!policy_manager_->SetExternalConsentStatus(external_consent_status)) { LOG4CXX_WARN(logger_, "External User Consent Settings status has not been set!"); @@ -667,6 +682,7 @@ void policy::PolicyHandler::SetDaysAfterEpoch() { #ifdef ENABLE_SECURITY std::string PolicyHandler::RetrieveCertificate() const { POLICY_LIB_CHECK(std::string("")); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->RetrieveCertificate(); } #endif // ENABLE_SECURITY @@ -675,19 +691,22 @@ void PolicyHandler::OnGetUserFriendlyMessage( const std::vector& message_codes, const std::string& language, uint32_t correlation_id) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); #ifdef EXTERNAL_PROPRIETARY_MODE const std::string active_hmi_language = application_manager::MessageHelper::CommonLanguageToString( application_manager_.hmi_capabilities().active_ui_language()); + policy_manager_lock_.Acquire(); const std::vector result = policy_manager_->GetUserFriendlyMessages( message_codes, language, active_hmi_language); + policy_manager_lock_.Release(); #else + policy_manager_lock_.Acquire(); const std::vector result = policy_manager_->GetUserFriendlyMessages(message_codes, language); + policy_manager_lock_.Release(); #endif // EXTERNAL_PROPRIETARY_MODE // Send response to HMI with gathered data MessageHelper::SendGetUserFriendlyMessageResponse( @@ -706,7 +725,6 @@ void PolicyHandler::GetRegisteredLinks( std::vector PolicyHandler::CollectRegisteredAppsPermissions() { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK(std::vector()); // If no specific app was passed, get permissions for all currently registered // applications @@ -718,6 +736,7 @@ PolicyHandler::CollectRegisteredAppsPermissions() { std::vector group_permissions; std::map::const_iterator it = app_to_device_link_.begin(); + sync_primitives::AutoLock lock_policy(policy_manager_lock_); for (; it != app_to_device_link_.end(); ++it) { policy_manager_->GetUserConsentForApp( it->first, it->second, group_permissions); @@ -751,6 +770,7 @@ std::vector PolicyHandler::CollectAppPermissions( return group_permissions; } + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->GetUserConsentForApp(device_params.device_mac_address, app->policy_app_id(), group_permissions); @@ -760,7 +780,6 @@ std::vector PolicyHandler::CollectAppPermissions( void PolicyHandler::OnGetListOfPermissions(const uint32_t connection_key, const uint32_t correlation_id) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); application_manager::ApplicationSharedPtr app = @@ -779,13 +798,19 @@ void PolicyHandler::OnGetListOfPermissions(const uint32_t connection_key, return; } - MessageHelper::SendGetListOfPermissionsResponse( - permissions, #ifdef EXTERNAL_PROPRIETARY_MODE - policy_manager_->GetExternalConsentStatus(), + policy_manager_lock_.Acquire(); + const ExternalConsentStatus external_consent_status = + policy_manager_->GetExternalConsentStatus(); + policy_manager_lock_.Release(); #endif // EXTERNAL_PROPRIETARY_MODE - correlation_id, - application_manager_); + + MessageHelper::SendGetListOfPermissionsResponse(permissions, +#ifdef EXTERNAL_PROPRIETARY_MODE + external_consent_status, +#endif // EXTERNAL_PROPRIETARY_MODE + correlation_id, + application_manager_); } void PolicyHandler::LinkAppsToDevice() { @@ -825,6 +850,7 @@ bool PolicyHandler::IsAppSuitableForPolicyUpdate( value->device(), application_manager_.connection_handler().get_session_observer()); + sync_primitives::AutoLock lock(policy_manager_lock_); const bool is_device_allowed = (kDeviceAllowed == policy_manager_->GetUserConsentForDevice( device_params.device_mac_address)); @@ -855,24 +881,27 @@ uint32_t PolicyHandler::ChooseRandomAppForPolicyUpdate( void PolicyHandler::OnDeviceSwitching(const std::string& device_id_from, const std::string& device_id_to) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->OnDeviceSwitching(device_id_from, device_id_to); } void PolicyHandler::OnGetStatusUpdate(const uint32_t correlation_id) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + policy_manager_lock_.Acquire(); + std::string policy_table_status = policy_manager_->GetPolicyTableStatus(); + policy_manager_lock_.Release(); + MessageHelper::SendGetStatusUpdateResponse( - policy_manager_->GetPolicyTableStatus(), - correlation_id, - application_manager_); + policy_table_status, correlation_id, application_manager_); } void PolicyHandler::OnUpdateStatusChanged(const std::string& status) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); - policy_manager_->SaveUpdateStatusRequired(policy::kUpToDate != status); + { + sync_primitives::AutoLock lock(policy_manager_lock_); + policy_manager_->SaveUpdateStatusRequired(policy::kUpToDate != status); + } MessageHelper::SendOnStatusUpdate(status, application_manager_); } @@ -898,21 +927,20 @@ std::string PolicyHandler::OnCurrentDeviceIdUpdateRequired( } void PolicyHandler::OnSystemInfoChanged(const std::string& language) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->SetSystemLanguage(language); } void PolicyHandler::OnGetSystemInfo(const std::string& ccpu_version, const std::string& wers_country_code, const std::string& language) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->SetSystemInfo(ccpu_version, wers_country_code, language); } void PolicyHandler::OnSystemInfoUpdateRequired() { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); MessageHelper::SendGetSystemInfoRequest(application_manager_); } @@ -940,6 +968,7 @@ void PolicyHandler::OnVehicleDataUpdated( return; } if (message[strings::msg_params].keyExists(strings::vin)) { + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->SetVINValue( message[strings::msg_params][strings::vin].asString()); } @@ -951,11 +980,10 @@ void PolicyHandler::OnVehicleDataUpdated( void PolicyHandler::OnPendingPermissionChange( const std::string& policy_app_id) { - LOG4CXX_AUTO_TRACE(logger_); + POLICY_LIB_CHECK_VOID(); LOG4CXX_DEBUG(logger_, "PolicyHandler::OnPendingPermissionChange for " << policy_app_id); - POLICY_LIB_CHECK_VOID(); ApplicationSharedPtr app = application_manager_.application_by_policy_id(policy_app_id); if (app.use_count() == 0) { @@ -964,8 +992,10 @@ void PolicyHandler::OnPendingPermissionChange( return; } + policy_manager_lock_.Acquire(); AppPermissions permissions = policy_manager_->GetAppPermissionsChanges(policy_app_id); + policy_manager_lock_.Release(); const uint32_t app_id = app->app_id(); @@ -978,6 +1008,7 @@ void PolicyHandler::OnPendingPermissionChange( mobile_apis::AudioStreamingState::NOT_AUDIBLE, mobile_apis::VideoStreamingState::NOT_STREAMABLE, true); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->RemovePendingPermissionChanges(policy_app_id); return; } @@ -1026,12 +1057,12 @@ void PolicyHandler::OnPendingPermissionChange( app->app_id(), permissions, application_manager_); } - policy_manager_->RemovePendingPermissionChanges(policy_app_id); + CallPolicyManagerFunction(&PolicyManager::RemovePendingPermissionChanges, + policy_app_id); } bool PolicyHandler::SendMessageToSDK(const BinaryMessage& pt_string, const std::string& url) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK(false); const uint32_t app_id = GetAppIdForSending(); @@ -1068,11 +1099,18 @@ bool PolicyHandler::ReceiveMessageFromSDK(const std::string& file, const BinaryMessage& pt_string) { POLICY_LIB_CHECK(false); - bool ret = policy_manager_->LoadPT(file, pt_string); + bool ret = false; + { + sync_primitives::AutoLock lock_policy(policy_manager_lock_); + ret = policy_manager_->LoadPT(file, pt_string); + } LOG4CXX_INFO(logger_, "Policy table is saved: " << std::boolalpha << ret); if (ret) { LOG4CXX_INFO(logger_, "PTU was successful."); - policy_manager_->CleanupUnpairedDevices(); + { + sync_primitives::AutoLock lock_policy(policy_manager_lock_); + policy_manager_->CleanupUnpairedDevices(); + } int32_t correlation_id = application_manager_.GetNextHMICorrelationID(); SetDaysAfterEpoch(); @@ -1095,8 +1133,8 @@ bool PolicyHandler::UnloadPolicyLibrary() { LOG4CXX_DEBUG(logger_, "policy_manager_ = " << policy_manager_); bool ret = true; AsyncRunner::Stop(); - sync_primitives::AutoWriteLock lock(policy_manager_lock_); if (policy_manager_) { + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_.reset(); } if (dl_handle_) { @@ -1111,9 +1149,11 @@ bool PolicyHandler::UnloadPolicyLibrary() { struct SDLAlowedNotification { SDLAlowedNotification(const connection_handler::DeviceHandle& device_id, PolicyManager* policy_manager, + sync_primitives::RecursiveLock& policy_manager_lock, StateController& state_controller) : device_id_(device_id) , policy_manager_(policy_manager) + , policy_manager_lock_(policy_manager_lock) , state_controller_(state_controller) {} void operator()(const ApplicationSharedPtr& app) { @@ -1121,7 +1161,10 @@ struct SDLAlowedNotification { if (app->device() == device_id_) { std::string hmi_level; mobile_apis::HMILevel::eType default_mobile_hmi; - policy_manager_->GetDefaultHmi(app->policy_app_id(), &hmi_level); + { + sync_primitives::AutoLock lock(policy_manager_lock_); + policy_manager_->GetDefaultHmi(app->policy_app_id(), &hmi_level); + } if ("BACKGROUND" == hmi_level) { default_mobile_hmi = mobile_apis::HMILevel::HMI_BACKGROUND; } else if ("FULL" == hmi_level) { @@ -1140,13 +1183,13 @@ struct SDLAlowedNotification { private: connection_handler::DeviceHandle device_id_; PolicyManager* policy_manager_; + sync_primitives::RecursiveLock& policy_manager_lock_; StateController& state_controller_; }; #endif // EXTERNAL_PROPRIETARY_MODE void PolicyHandler::OnAllowSDLFunctionalityNotification( bool is_allowed, const std::string& device_mac) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); using namespace mobile_apis; const bool device_specific = !device_mac.empty(); @@ -1168,7 +1211,11 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( "Device with id " << device_id << " wasn't found."); continue; } + + policy_manager_lock_.Acquire(); policy_manager_->SetUserConsentForDevice(device_id, is_allowed); + policy_manager_lock_.Release(); + connection_handler::DeviceHandle device_handle = 0; if (!connection_handler.GetDeviceID(device_id, &device_handle)) { LOG4CXX_WARN(logger_, @@ -1192,6 +1239,7 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( accessor.GetData().end(), SDLAlowedNotification(device_handle, policy_manager_.get(), + policy_manager_lock_, application_manager_.state_controller())); } #endif // EXTERNAL_PROPRIETARY_MODE @@ -1201,7 +1249,10 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( // Case, when specific device was changed connection_handler::DeviceHandle device_handle = 0u; if (device_specific) { - policy_manager_->SetUserConsentForDevice(device_mac, is_allowed); + { + sync_primitives::AutoLock lock(policy_manager_lock_); + policy_manager_->SetUserConsentForDevice(device_mac, is_allowed); + } if (!connection_handler.GetDeviceID(device_mac, &device_handle)) { LOG4CXX_WARN(logger_, "Device hadle with mac " << device_mac << " wasn't found."); @@ -1254,8 +1305,8 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( } void PolicyHandler::OnIgnitionCycleOver() { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->IncrementIgnitionCycles(); } @@ -1271,14 +1322,16 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, AppPermissions permissions(policy_app_id); - sync_primitives::AutoReadLock lock(policy_manager_lock_); if (!policy_manager_) { LOG4CXX_DEBUG(logger_, "The shared library of policy is not loaded"); if (!PolicyEnabled()) { permissions.isSDLAllowed = true; } } else { - permissions = policy_manager_->GetAppPermissionsChanges(policy_app_id); + { + sync_primitives::AutoLock lock(policy_manager_lock_); + permissions = policy_manager_->GetAppPermissionsChanges(policy_app_id); + } #ifdef EXTERNAL_PROPRIETARY_MODE UsageStatistics& usage = app->usage_report(); @@ -1288,8 +1341,11 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, application_manager_.connection_handler().get_session_observer()); permissions.deviceInfo = device_params; + policy_manager_lock_.Acquire(); DeviceConsent consent = policy_manager_->GetUserConsentForDevice( permissions.deviceInfo.device_mac_address); + policy_manager_lock_.Release(); + permissions.isSDLAllowed = kDeviceAllowed == consent; // According to the SDLAQ-CRS-2794, p.9 @@ -1317,6 +1373,7 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, #else // EXTERNAL_PROPRIETARY_MODE permissions.isSDLAllowed = true; #endif // EXTERNAL_PROPRIETARY_MODE + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->RemovePendingPermissionChanges(policy_app_id); } // If application is revoked it should not be activated @@ -1335,16 +1392,20 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, } void PolicyHandler::KmsChanged(int kilometers) { + POLICY_LIB_CHECK_VOID(); LOG4CXX_DEBUG(logger_, "PolicyHandler::KmsChanged " << kilometers << " kilometers"); - POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->KmsChanged(kilometers); } void PolicyHandler::PTExchangeAtUserRequest(uint32_t correlation_id) { - LOG4CXX_TRACE(logger_, "PT exchange at user request"); POLICY_LIB_CHECK_VOID(); + LOG4CXX_DEBUG(logger_, "PT exchange at user request"); + policy_manager_lock_.Acquire(); std::string update_status = policy_manager_->ForcePTExchangeAtUserRequest(); + policy_manager_lock_.Release(); + MessageHelper::SendUpdateSDLResponse( update_status, correlation_id, application_manager_); } @@ -1462,7 +1523,6 @@ void PolicyHandler::OnSnapshotCreated( } #else // EXTERNAL_PROPRIETARY_MODE void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); #ifdef PROPRIETARY_MODE std::string policy_snapshot_full_path; @@ -1477,17 +1537,23 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string) { #else // PROPRIETARY_MODE LOG4CXX_ERROR(logger_, "HTTP policy"); EndpointUrls urls; - policy_manager_->GetUpdateUrls("0x07", urls); + { + sync_primitives::AutoLock lock(policy_manager_lock_); + policy_manager_->GetUpdateUrls("0x07", urls); + } if (urls.empty()) { LOG4CXX_ERROR(logger_, "Service URLs are empty! NOT sending PT to mobile!"); return; } + policy_manager_lock_.Acquire(); AppIdURL app_url = policy_manager_->GetNextUpdateUrl(urls); while (!IsUrlAppIdValid(app_url.first, urls)) { app_url = policy_manager_->GetNextUpdateUrl(urls); } + policy_manager_lock_.Release(); + const std::string& url = urls[app_url.first].url[app_url.second]; SendMessageToSDK(pt_string, url); #endif // PROPRIETARY_MODE @@ -1499,6 +1565,7 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string) { bool PolicyHandler::GetPriority(const std::string& policy_app_id, std::string* priority) const { POLICY_LIB_CHECK(false); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetPriority(policy_app_id, priority); } @@ -1523,6 +1590,8 @@ void PolicyHandler::CheckPermissions( "Checking permissions for " << app->policy_app_id() << " in " << hmi_level << " on device " << device_id << " rpc " << rpc); + + sync_primitives::AutoLock lock(policy_manager_lock_); #ifdef EXTERNAL_PROPRIETARY_MODE policy_manager_->CheckPermissions( app->policy_app_id(), hmi_level, rpc, rpc_params, result); @@ -1535,18 +1604,21 @@ void PolicyHandler::CheckPermissions( uint32_t PolicyHandler::GetNotificationsNumber( const std::string& priority) const { POLICY_LIB_CHECK(0); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetNotificationsNumber(priority); } DeviceConsent PolicyHandler::GetUserConsentForDevice( const std::string& device_id) const { POLICY_LIB_CHECK(kDeviceDisallowed); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetUserConsentForDevice(device_id); } bool PolicyHandler::GetDefaultHmi(const std::string& policy_app_id, std::string* default_hmi) const { POLICY_LIB_CHECK(false); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetDefaultHmi(policy_app_id, default_hmi); } @@ -1554,6 +1626,7 @@ bool PolicyHandler::GetInitialAppData(const std::string& application_id, StringArray* nicknames, StringArray* app_hmi_types) { POLICY_LIB_CHECK(false); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetInitialAppData( application_id, nicknames, app_hmi_types); } @@ -1561,23 +1634,26 @@ bool PolicyHandler::GetInitialAppData(const std::string& application_id, void PolicyHandler::GetUpdateUrls(const std::string& service_type, EndpointUrls& out_end_points) { POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->GetUpdateUrls(service_type, out_end_points); } void PolicyHandler::GetUpdateUrls(const uint32_t service_type, EndpointUrls& out_end_points) { POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->GetUpdateUrls(service_type, out_end_points); } std::string PolicyHandler::GetLockScreenIconUrl() const { POLICY_LIB_CHECK(std::string("")); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetLockScreenIconUrl(); } uint32_t PolicyHandler::NextRetryTimeout() { POLICY_LIB_CHECK(0); - LOG4CXX_AUTO_TRACE(logger_); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->NextRetryTimeout(); } @@ -1587,21 +1663,25 @@ uint32_t PolicyHandler::TimeoutExchangeSec() const { uint32_t PolicyHandler::TimeoutExchangeMSec() const { POLICY_LIB_CHECK(0); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->TimeoutExchangeMSec(); } void PolicyHandler::OnExceededTimeout() { POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->OnExceededTimeout(); } void PolicyHandler::OnSystemReady() { POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->OnSystemReady(); } void PolicyHandler::PTUpdatedAt(Counters counter, int value) { POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->PTUpdatedAt(counter, value); } @@ -1721,7 +1801,6 @@ void PolicyHandler::OnEmptyCertificateArrived() const { } void PolicyHandler::OnCertificateDecrypted(bool is_succeeded) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); const std::string file_name = @@ -1748,8 +1827,10 @@ void PolicyHandler::OnCertificateDecrypted(bool is_succeeded) { LOG4CXX_DEBUG(logger_, "Loaded decrypted certificate data: \"" << certificate_data << '"'); - - policy_manager_->SetDecryptedCertificate(certificate_data); + { + sync_primitives::AutoLock lock(policy_manager_lock_); + policy_manager_->SetDecryptedCertificate(certificate_data); + } sync_primitives::AutoLock lock(listeners_lock_); std::for_each( @@ -1786,10 +1867,13 @@ bool PolicyHandler::CanUpdate() { } void PolicyHandler::RemoveDevice(const std::string& device_id) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); - policy_manager_->MarkUnpairedDevice(device_id); + { + sync_primitives::AutoLock lock(policy_manager_lock_); + policy_manager_->MarkUnpairedDevice(device_id); + } + #ifdef EXTERNAL_PROPRIETARY_MODE connection_handler::DeviceHandle device_uid; if (application_manager_.connection_handler().GetDeviceID(device_id, @@ -1804,25 +1888,26 @@ void PolicyHandler::RemoveDevice(const std::string& device_id) { } bool PolicyHandler::IsApplicationRevoked(const std::string& app_id) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK(false); - + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->IsApplicationRevoked(app_id); } void PolicyHandler::OnUpdateRequestSentToMobile() { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->OnUpdateStarted(); } bool PolicyHandler::CheckKeepContext(const std::string& policy_app_id) const { POLICY_LIB_CHECK(false); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->CanAppKeepContext(policy_app_id); } bool PolicyHandler::CheckStealFocus(const std::string& policy_app_id) const { POLICY_LIB_CHECK(false); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->CanAppStealFocus(policy_app_id); } @@ -1847,6 +1932,7 @@ bool PolicyHandler::CheckSystemAction( uint32_t PolicyHandler::HeartBeatTimeout(const std::string& app_id) const { POLICY_LIB_CHECK(0); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->HeartBeatTimeout(app_id); } @@ -1854,6 +1940,7 @@ const std::string PolicyHandler::RemoteAppsUrl() const { const std::string default_url; POLICY_LIB_CHECK(default_url); EndpointUrls endpoints; + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->GetUpdateUrls("queryAppsUrl", endpoints); if (endpoints.empty() || endpoints[0].url.empty()) { return default_url; @@ -1864,28 +1951,33 @@ const std::string PolicyHandler::RemoteAppsUrl() const { void PolicyHandler::OnAppsSearchStarted() { POLICY_LIB_CHECK(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->OnAppsSearchStarted(); } void PolicyHandler::OnAppsSearchCompleted(const bool trigger_ptu) { POLICY_LIB_CHECK(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->OnAppsSearchCompleted(trigger_ptu); } void PolicyHandler::OnAppRegisteredOnMobile(const std::string& application_id) { POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->OnAppRegisteredOnMobile(application_id); } RequestType::State PolicyHandler::GetAppRequestTypeState( const std::string& policy_app_id) const { POLICY_LIB_CHECK(RequestType::State::UNAVAILABLE); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetAppRequestTypesState(policy_app_id); } RequestSubType::State PolicyHandler::GetAppRequestSubTypeState( const std::string& policy_app_id) const { POLICY_LIB_CHECK(RequestSubType::State::UNAVAILABLE); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetAppRequestSubTypesState(policy_app_id); } @@ -1917,8 +2009,11 @@ bool PolicyHandler::IsRequestTypeAllowed( } case RequestType::State::AVAILABLE: { // If any of request types is available for current application - get them + policy_manager_lock_.Acquire(); const auto request_types = policy_manager_->GetAppRequestTypes(policy_app_id); + policy_manager_lock_.Release(); + return helpers::in_range(request_types, stringified_type); } default: @@ -1937,8 +2032,11 @@ bool PolicyHandler::IsRequestSubTypeAllowed( return false; } + policy_manager_lock_.Acquire(); const RequestSubType::State request_subtype_state = policy_manager_->GetAppRequestSubTypesState(policy_app_id); + policy_manager_lock_.Release(); + switch (request_subtype_state) { case RequestSubType::State::EMPTY: { // If empty array of request subtypes is assigned to app - any is allowed @@ -1954,8 +2052,11 @@ bool PolicyHandler::IsRequestSubTypeAllowed( case RequestSubType::State::AVAILABLE: { // If any of request subtypes is available for current application // get them all + policy_manager_lock_.Acquire(); const auto request_subtypes = policy_manager_->GetAppRequestSubTypes(policy_app_id); + policy_manager_lock_.Release(); + return helpers::in_range(request_subtypes, request_subtype); } default: @@ -1966,6 +2067,7 @@ bool PolicyHandler::IsRequestSubTypeAllowed( const std::vector PolicyHandler::GetAppRequestTypes( const std::string& policy_app_id) const { POLICY_LIB_CHECK(std::vector()); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetAppRequestTypes(policy_app_id); } @@ -1977,24 +2079,28 @@ const std::vector PolicyHandler::GetAppRequestSubTypes( const VehicleInfo policy::PolicyHandler::GetVehicleInfo() const { POLICY_LIB_CHECK(VehicleInfo()); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetVehicleInfo(); } #ifdef EXTERNAL_PROPRIETARY_MODE const MetaInfo PolicyHandler::GetMetaInfo() const { POLICY_LIB_CHECK(MetaInfo()); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetMetaInfo(); } #endif // EXTERNAL_PROPRIETARY_MODE void PolicyHandler::Increment(usage_statistics::GlobalCounterId type) { POLICY_LIB_CHECK(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->Increment(type); } void PolicyHandler::Increment(const std::string& app_id, usage_statistics::AppCounterId type) { POLICY_LIB_CHECK(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->Increment(app_id, type); } @@ -2002,6 +2108,7 @@ void PolicyHandler::Set(const std::string& app_id, usage_statistics::AppInfoId type, const std::string& value) { POLICY_LIB_CHECK(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->Set(app_id, type, value); } @@ -2009,6 +2116,7 @@ void PolicyHandler::Add(const std::string& app_id, usage_statistics::AppStopwatchId type, int32_t timespan_seconds) { POLICY_LIB_CHECK(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->Add(app_id, type, timespan_seconds); } @@ -2055,12 +2163,14 @@ void PolicyHandler::UpdateHMILevel(ApplicationSharedPtr app, bool PolicyHandler::CheckModule(const PTString& app_id, const PTString& module) { POLICY_LIB_CHECK(false); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->CheckModule(app_id, module); } void PolicyHandler::OnRemoteAppPermissionsChanged( const std::string& device_id, const std::string& application_id) { POLICY_LIB_CHECK_VOID(); + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->SendAppPermissionsChanged(device_id, application_id); } @@ -2095,15 +2205,14 @@ void PolicyHandler::OnUpdateHMIStatus(const std::string& device_id, bool PolicyHandler::GetModuleTypes(const std::string& policy_app_id, std::vector* modules) const { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK(false); + sync_primitives::AutoLock lock(policy_manager_lock_); return policy_manager_->GetModuleTypes(policy_app_id, modules); } void PolicyHandler::SetDefaultHmiTypes( const std::string& application_id, const smart_objects::SmartObject* app_types) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); std::vector hmi_types; if (app_types && app_types->asArray()) { @@ -2113,16 +2222,20 @@ void PolicyHandler::SetDefaultHmiTypes( std::back_inserter(hmi_types), SmartObjectToInt()); } + + sync_primitives::AutoLock lock(policy_manager_lock_); policy_manager_->SetDefaultHmiTypes(application_id, hmi_types); } bool PolicyHandler::CheckHMIType(const std::string& application_id, mobile_apis::AppHMIType::eType hmi, const smart_objects::SmartObject* app_types) { - LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK(false); std::vector policy_hmi_types; + + policy_manager_lock_.Acquire(); bool ret = policy_manager_->GetHMITypes(application_id, &policy_hmi_types); + policy_manager_lock_.Release(); std::vector additional_hmi_types; if (app_types && app_types->asArray()) { From 8dd6d43a16fb97780a1ec5d1a05b663952be029d Mon Sep 17 00:00:00 2001 From: sniukalov Date: Fri, 9 Nov 2018 13:35:14 +0200 Subject: [PATCH 2/4] Removed direct use of the "acquire/release" of the synchronization object. --- .../policies/policy_handler.h | 11 ++ .../src/policies/policy_handler.cc | 151 ++++++++---------- 2 files changed, 75 insertions(+), 87 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 4e544e0687c..fa041bfdd68 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 @@ -737,6 +737,17 @@ class PolicyHandler : public PolicyHandlerInterface, std::shared_ptr event_observer_; uint32_t last_activated_app_id_; + /** + * @brief Call PolicyManager function with sync_primitives::AutoLock + * @param func function PolicyManager to call + * @param args arguments for call function + * @return result PolicyManager function + */ + template + auto CallPolicyManagerFunction(Func func, Args... args) const + -> decltype((std::declval().* + std::declval())(std::declval()...)); + /** * @brief Contains device handles, which were sent for user consent to HMI */ diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index a5cd3ee14a4..6bf38699f7a 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -358,9 +358,7 @@ bool PolicyHandler::LoadPolicyLibrary() { const char* error = dlerror(); if (!error) { if (CreateManager()) { - policy_manager_lock_.Acquire(); - policy_manager_->set_listener(this); - policy_manager_lock_.Release(); + CallPolicyManagerFunction(&PolicyManager::set_listener, this); event_observer_ = std::shared_ptr(new PolicyEventObserver( @@ -649,13 +647,13 @@ void PolicyHandler::OnAppPermissionConsentInternal( out_permissions.policy_app_id = it->second; out_permissions.device_id = it->first; - policy_manager_lock_.Acquire(); #ifdef EXTERNAL_PROPRIETARY_MODE - policy_manager_->SetUserConsentForApp(out_permissions, mode); + CallPolicyManagerFunction( + &PolicyManager::SetUserConsentForApp, out_permissions, mode); #else - policy_manager_->SetUserConsentForApp(out_permissions); + CallPolicyManagerFunction(&PolicyManager::SetUserConsentForApp, + out_permissions); #endif - policy_manager_lock_.Release(); } } else { LOG4CXX_WARN(logger_, @@ -697,16 +695,14 @@ void PolicyHandler::OnGetUserFriendlyMessage( const std::string active_hmi_language = application_manager::MessageHelper::CommonLanguageToString( application_manager_.hmi_capabilities().active_ui_language()); - policy_manager_lock_.Acquire(); const std::vector result = - policy_manager_->GetUserFriendlyMessages( - message_codes, language, active_hmi_language); - policy_manager_lock_.Release(); + CallPolicyManagerFunction(&PolicyManager::GetUserFriendlyMessages, + message_codes, + language, + active_hmi_language); #else - policy_manager_lock_.Acquire(); - const std::vector result = - policy_manager_->GetUserFriendlyMessages(message_codes, language); - policy_manager_lock_.Release(); + const std::vector result = CallPolicyManagerFunction( + &PolicyManager::GetUserFriendlyMessages, message_codes, language); #endif // EXTERNAL_PROPRIETARY_MODE // Send response to HMI with gathered data MessageHelper::SendGetUserFriendlyMessageResponse( @@ -799,10 +795,8 @@ void PolicyHandler::OnGetListOfPermissions(const uint32_t connection_key, } #ifdef EXTERNAL_PROPRIETARY_MODE - policy_manager_lock_.Acquire(); const ExternalConsentStatus external_consent_status = - policy_manager_->GetExternalConsentStatus(); - policy_manager_lock_.Release(); + CallPolicyManagerFunction(&PolicyManager::GetExternalConsentStatus); #endif // EXTERNAL_PROPRIETARY_MODE MessageHelper::SendGetListOfPermissionsResponse(permissions, @@ -888,9 +882,8 @@ void PolicyHandler::OnDeviceSwitching(const std::string& device_id_from, void PolicyHandler::OnGetStatusUpdate(const uint32_t correlation_id) { POLICY_LIB_CHECK_VOID(); - policy_manager_lock_.Acquire(); - std::string policy_table_status = policy_manager_->GetPolicyTableStatus(); - policy_manager_lock_.Release(); + std::string policy_table_status = + CallPolicyManagerFunction(&PolicyManager::GetPolicyTableStatus); MessageHelper::SendGetStatusUpdateResponse( policy_table_status, correlation_id, application_manager_); @@ -898,10 +891,10 @@ void PolicyHandler::OnGetStatusUpdate(const uint32_t correlation_id) { void PolicyHandler::OnUpdateStatusChanged(const std::string& status) { POLICY_LIB_CHECK_VOID(); - { - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SaveUpdateStatusRequired(policy::kUpToDate != status); - } + + CallPolicyManagerFunction(&PolicyManager::SaveUpdateStatusRequired, + policy::kUpToDate != status); + MessageHelper::SendOnStatusUpdate(status, application_manager_); } @@ -992,10 +985,8 @@ void PolicyHandler::OnPendingPermissionChange( return; } - policy_manager_lock_.Acquire(); - AppPermissions permissions = - policy_manager_->GetAppPermissionsChanges(policy_app_id); - policy_manager_lock_.Release(); + AppPermissions permissions = CallPolicyManagerFunction( + &PolicyManager::GetAppPermissionsChanges, policy_app_id); const uint32_t app_id = app->app_id(); @@ -1099,18 +1090,14 @@ bool PolicyHandler::ReceiveMessageFromSDK(const std::string& file, const BinaryMessage& pt_string) { POLICY_LIB_CHECK(false); - bool ret = false; - { - sync_primitives::AutoLock lock_policy(policy_manager_lock_); - ret = policy_manager_->LoadPT(file, pt_string); - } + bool ret = CallPolicyManagerFunction(&PolicyManager::LoadPT, file, pt_string); + LOG4CXX_INFO(logger_, "Policy table is saved: " << std::boolalpha << ret); if (ret) { LOG4CXX_INFO(logger_, "PTU was successful."); - { - sync_primitives::AutoLock lock_policy(policy_manager_lock_); - policy_manager_->CleanupUnpairedDevices(); - } + + CallPolicyManagerFunction(&PolicyManager::CleanupUnpairedDevices); + int32_t correlation_id = application_manager_.GetNextHMICorrelationID(); SetDaysAfterEpoch(); @@ -1212,9 +1199,8 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( continue; } - policy_manager_lock_.Acquire(); - policy_manager_->SetUserConsentForDevice(device_id, is_allowed); - policy_manager_lock_.Release(); + CallPolicyManagerFunction( + &PolicyManager::SetUserConsentForDevice, device_id, is_allowed); connection_handler::DeviceHandle device_handle = 0; if (!connection_handler.GetDeviceID(device_id, &device_handle)) { @@ -1249,10 +1235,9 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( // Case, when specific device was changed connection_handler::DeviceHandle device_handle = 0u; if (device_specific) { - { - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SetUserConsentForDevice(device_mac, is_allowed); - } + CallPolicyManagerFunction( + &PolicyManager::SetUserConsentForDevice, device_mac, is_allowed); + if (!connection_handler.GetDeviceID(device_mac, &device_handle)) { LOG4CXX_WARN(logger_, "Device hadle with mac " << device_mac << " wasn't found."); @@ -1328,10 +1313,9 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, permissions.isSDLAllowed = true; } } else { - { - sync_primitives::AutoLock lock(policy_manager_lock_); - permissions = policy_manager_->GetAppPermissionsChanges(policy_app_id); - } + permissions = CallPolicyManagerFunction( + &PolicyManager::GetAppPermissionsChanges, policy_app_id); + #ifdef EXTERNAL_PROPRIETARY_MODE UsageStatistics& usage = app->usage_report(); @@ -1341,10 +1325,9 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, application_manager_.connection_handler().get_session_observer()); permissions.deviceInfo = device_params; - policy_manager_lock_.Acquire(); - DeviceConsent consent = policy_manager_->GetUserConsentForDevice( - permissions.deviceInfo.device_mac_address); - policy_manager_lock_.Release(); + DeviceConsent consent = + CallPolicyManagerFunction(&PolicyManager::GetUserConsentForDevice, + permissions.deviceInfo.device_mac_address); permissions.isSDLAllowed = kDeviceAllowed == consent; @@ -1402,9 +1385,9 @@ void PolicyHandler::KmsChanged(int kilometers) { void PolicyHandler::PTExchangeAtUserRequest(uint32_t correlation_id) { POLICY_LIB_CHECK_VOID(); LOG4CXX_DEBUG(logger_, "PT exchange at user request"); - policy_manager_lock_.Acquire(); - std::string update_status = policy_manager_->ForcePTExchangeAtUserRequest(); - policy_manager_lock_.Release(); + + std::string update_status = + CallPolicyManagerFunction(&PolicyManager::ForcePTExchangeAtUserRequest); MessageHelper::SendUpdateSDLResponse( update_status, correlation_id, application_manager_); @@ -1537,22 +1520,19 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string) { #else // PROPRIETARY_MODE LOG4CXX_ERROR(logger_, "HTTP policy"); EndpointUrls urls; - { - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->GetUpdateUrls("0x07", urls); - } + + CallPolicyManagerFunction(&PolicyManager::GetUpdateUrls, "0x07", urls); if (urls.empty()) { LOG4CXX_ERROR(logger_, "Service URLs are empty! NOT sending PT to mobile!"); return; } - policy_manager_lock_.Acquire(); - AppIdURL app_url = policy_manager_->GetNextUpdateUrl(urls); + AppIdURL app_url = + CallPolicyManagerFunction(&PolicyManager::GetNextUpdateUrl, urls); while (!IsUrlAppIdValid(app_url.first, urls)) { - app_url = policy_manager_->GetNextUpdateUrl(urls); + app_url = CallPolicyManagerFunction(&PolicyManager::GetNextUpdateUrl, urls); } - policy_manager_lock_.Release(); const std::string& url = urls[app_url.first].url[app_url.second]; SendMessageToSDK(pt_string, url); @@ -1827,10 +1807,8 @@ void PolicyHandler::OnCertificateDecrypted(bool is_succeeded) { LOG4CXX_DEBUG(logger_, "Loaded decrypted certificate data: \"" << certificate_data << '"'); - { - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SetDecryptedCertificate(certificate_data); - } + CallPolicyManagerFunction(&PolicyManager::SetDecryptedCertificate, + certificate_data); sync_primitives::AutoLock lock(listeners_lock_); std::for_each( @@ -1869,10 +1847,7 @@ bool PolicyHandler::CanUpdate() { void PolicyHandler::RemoveDevice(const std::string& device_id) { POLICY_LIB_CHECK_VOID(); - { - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->MarkUnpairedDevice(device_id); - } + CallPolicyManagerFunction(&PolicyManager::MarkUnpairedDevice, device_id); #ifdef EXTERNAL_PROPRIETARY_MODE connection_handler::DeviceHandle device_uid; @@ -2009,10 +1984,8 @@ bool PolicyHandler::IsRequestTypeAllowed( } case RequestType::State::AVAILABLE: { // If any of request types is available for current application - get them - policy_manager_lock_.Acquire(); - const auto request_types = - policy_manager_->GetAppRequestTypes(policy_app_id); - policy_manager_lock_.Release(); + const auto request_types = CallPolicyManagerFunction( + &PolicyManager::GetAppRequestTypes, policy_app_id); return helpers::in_range(request_types, stringified_type); } @@ -2032,10 +2005,8 @@ bool PolicyHandler::IsRequestSubTypeAllowed( return false; } - policy_manager_lock_.Acquire(); - const RequestSubType::State request_subtype_state = - policy_manager_->GetAppRequestSubTypesState(policy_app_id); - policy_manager_lock_.Release(); + const RequestSubType::State request_subtype_state = CallPolicyManagerFunction( + &PolicyManager::GetAppRequestSubTypesState, policy_app_id); switch (request_subtype_state) { case RequestSubType::State::EMPTY: { @@ -2052,10 +2023,8 @@ bool PolicyHandler::IsRequestSubTypeAllowed( case RequestSubType::State::AVAILABLE: { // If any of request subtypes is available for current application // get them all - policy_manager_lock_.Acquire(); - const auto request_subtypes = - policy_manager_->GetAppRequestSubTypes(policy_app_id); - policy_manager_lock_.Release(); + const auto request_subtypes = CallPolicyManagerFunction( + &PolicyManager::GetAppRequestSubTypes, policy_app_id); return helpers::in_range(request_subtypes, request_subtype); } @@ -2233,9 +2202,8 @@ bool PolicyHandler::CheckHMIType(const std::string& application_id, POLICY_LIB_CHECK(false); std::vector policy_hmi_types; - policy_manager_lock_.Acquire(); - bool ret = policy_manager_->GetHMITypes(application_id, &policy_hmi_types); - policy_manager_lock_.Release(); + bool ret = CallPolicyManagerFunction( + &PolicyManager::GetHMITypes, application_id, &policy_hmi_types); std::vector additional_hmi_types; if (app_types && app_types->asArray()) { @@ -2272,4 +2240,13 @@ void PolicyHandler::OnUpdateHMILevel(const std::string& device_id, } UpdateHMILevel(app, level); } + +template +auto PolicyHandler::CallPolicyManagerFunction(Func func, Args... args) const + -> decltype((std::declval().* + std::declval())(std::declval()...)) { + sync_primitives::AutoLock lock(policy_manager_lock_); + return ((*policy_manager_).*func)(args...); +} + } // namespace policy From 741b392f66d928d4ee275bab0e7759a502843385 Mon Sep 17 00:00:00 2001 From: sniukalov Date: Fri, 9 Nov 2018 13:36:07 +0200 Subject: [PATCH 3/4] Minimized use of policy_manager_lock_. --- .../policies/policy_handler.h | 7 +- .../src/policies/policy_handler.cc | 229 ++++++++---------- 2 files changed, 110 insertions(+), 126 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 fa041bfdd68..24efe8cd1bd 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 @@ -744,9 +744,12 @@ class PolicyHandler : public PolicyHandlerInterface, * @return result PolicyManager function */ template - auto CallPolicyManagerFunction(Func func, Args... args) const + auto CallPolicyManagerFunction(Func func, Args&&... args) const -> decltype((std::declval().* - std::declval())(std::declval()...)); + std::declval())(std::declval()...)) { + sync_primitives::AutoLock lock(policy_manager_lock_); + return ((*policy_manager_).*func)(args...); + } /** * @brief Contains device handles, which were sent for user consent to HMI diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 6bf38699f7a..2d6b220e560 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -125,7 +125,7 @@ struct HMILevelPredicate : level_(level) {} bool operator()(const ApplicationSharedPtr app) const { - return level_ == app->hmi_level() ? true : false; + return (app && (level_ == app->hmi_level())) ? true : false; } private: @@ -161,7 +161,7 @@ struct DeactivateApplication { : device_id_(device_id), state_ctrl_(state_ctrl) {} void operator()(const ApplicationSharedPtr& app) { - if (device_id_ == app->device()) { + if (app && (device_id_ == app->device())) { state_ctrl_.SetRegularState( app, mobile_apis::HMILevel::HMI_NONE, @@ -188,7 +188,7 @@ struct SDLAllowedNotification { void operator()(const ApplicationSharedPtr& app) { DCHECK_OR_RETURN_VOID(policy_manager_); - if (device_id_ == app->device()) { + if (app && (device_id_ == app->device())) { std::string hmi_level = "NONE"; mobile_apis::HMILevel::eType default_mobile_hmi; { @@ -400,8 +400,8 @@ bool PolicyHandler::InitPolicyTable() { hmi_apis::FunctionID::BasicCommunication_OnReady); std::string preloaded_file = get_settings().preloaded_pt_file(); if (file_system::FileExists(preloaded_file)) { - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->InitPT(preloaded_file, &get_settings()); + return CallPolicyManagerFunction( + &PolicyManager::InitPT, preloaded_file, &get_settings()); } LOG4CXX_FATAL(logger_, "The file which contains preloaded PT is not exist"); return false; @@ -411,8 +411,7 @@ bool PolicyHandler::ResetPolicyTable() { POLICY_LIB_CHECK(false); std::string preloaded_file = get_settings().preloaded_pt_file(); if (file_system::FileExists(preloaded_file)) { - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->ResetPT(preloaded_file); + return CallPolicyManagerFunction(&PolicyManager::ResetPT, preloaded_file); } LOG4CXX_WARN(logger_, "The file which contains preloaded PT is not exist"); return false; @@ -420,8 +419,7 @@ bool PolicyHandler::ResetPolicyTable() { bool PolicyHandler::ClearUserConsent() { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->ResetUserConsent(); + return CallPolicyManagerFunction(&PolicyManager::ResetUserConsent); } uint32_t PolicyHandler::GetAppIdForSending() const { @@ -537,8 +535,7 @@ void PolicyHandler::SendOnAppPermissionsChanged( void PolicyHandler::OnPTExchangeNeeded() { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->ForcePTExchange(); + CallPolicyManagerFunction(&PolicyManager::ForcePTExchange); } void PolicyHandler::GetAvailableApps(std::queue& apps) { @@ -563,22 +560,22 @@ StatusNotifier PolicyHandler::AddApplication( const std::string& application_id, const rpc::policy_table_interface_base::AppHmiTypes& hmi_types) { POLICY_LIB_CHECK(std::make_shared()); - sync_primitives::AutoLock lock_policy(policy_manager_lock_); - return policy_manager_->AddApplication(application_id, hmi_types); + return CallPolicyManagerFunction( + &PolicyManager::AddApplication, application_id, hmi_types); } void PolicyHandler::AddDevice(const std::string& device_id, const std::string& connection_type) { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->AddDevice(device_id, connection_type); + CallPolicyManagerFunction( + &PolicyManager::AddDevice, device_id, connection_type); } void PolicyHandler::SetDeviceInfo(const std::string& device_id, const DeviceInfo& device_info) { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SetDeviceInfo(device_id, device_info); + CallPolicyManagerFunction( + &PolicyManager::SetDeviceInfo, device_id, device_info); } #ifdef EXTERNAL_PROPRIETARY_MODE @@ -608,11 +605,12 @@ void PolicyHandler::OnAppPermissionConsentInternal( } if (!out_permissions.policy_app_id.empty()) { - sync_primitives::AutoLock lock(policy_manager_lock_); #ifdef EXTERNAL_PROPRIETARY_MODE - policy_manager_->SetUserConsentForApp(out_permissions, mode); + CallPolicyManagerFunction( + &PolicyManager::SetUserConsentForApp, out_permissions, mode); #else - policy_manager_->SetUserConsentForApp(out_permissions); + CallPolicyManagerFunction(&PolicyManager::SetUserConsentForApp, + out_permissions); #endif } } else if (!app_to_device_link_.empty()) { @@ -661,8 +659,8 @@ void PolicyHandler::OnAppPermissionConsentInternal( "setting common permissions."); } #ifdef EXTERNAL_PROPRIETARY_MODE - sync_primitives::AutoLock lock(policy_manager_lock_); - if (!policy_manager_->SetExternalConsentStatus(external_consent_status)) { + if (!CallPolicyManagerFunction(&PolicyManager::SetExternalConsentStatus, + external_consent_status)) { LOG4CXX_WARN(logger_, "External User Consent Settings status has not been set!"); } @@ -680,8 +678,7 @@ void policy::PolicyHandler::SetDaysAfterEpoch() { #ifdef ENABLE_SECURITY std::string PolicyHandler::RetrieveCertificate() const { POLICY_LIB_CHECK(std::string("")); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->RetrieveCertificate(); + return CallPolicyManagerFunction(&PolicyManager::RetrieveCertificate); } #endif // ENABLE_SECURITY @@ -766,10 +763,10 @@ std::vector PolicyHandler::CollectAppPermissions( return group_permissions; } - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->GetUserConsentForApp(device_params.device_mac_address, - app->policy_app_id(), - group_permissions); + CallPolicyManagerFunction(&PolicyManager::GetUserConsentForApp, + device_params.device_mac_address, + app->policy_app_id(), + group_permissions); return group_permissions; } @@ -844,10 +841,10 @@ bool PolicyHandler::IsAppSuitableForPolicyUpdate( value->device(), application_manager_.connection_handler().get_session_observer()); - sync_primitives::AutoLock lock(policy_manager_lock_); - const bool is_device_allowed = (kDeviceAllowed == - policy_manager_->GetUserConsentForDevice( - device_params.device_mac_address)); + const bool is_device_allowed = + (kDeviceAllowed == + CallPolicyManagerFunction(&PolicyManager::GetUserConsentForDevice, + device_params.device_mac_address)); LOG4CXX_DEBUG(logger_, "Is device " << device_params.device_mac_address << " allowed " @@ -876,8 +873,8 @@ uint32_t PolicyHandler::ChooseRandomAppForPolicyUpdate( void PolicyHandler::OnDeviceSwitching(const std::string& device_id_from, const std::string& device_id_to) { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->OnDeviceSwitching(device_id_from, device_id_to); + CallPolicyManagerFunction( + &PolicyManager::OnDeviceSwitching, device_id_from, device_id_to); } void PolicyHandler::OnGetStatusUpdate(const uint32_t correlation_id) { @@ -921,16 +918,15 @@ std::string PolicyHandler::OnCurrentDeviceIdUpdateRequired( void PolicyHandler::OnSystemInfoChanged(const std::string& language) { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SetSystemLanguage(language); + CallPolicyManagerFunction(&PolicyManager::SetSystemLanguage, language); } void PolicyHandler::OnGetSystemInfo(const std::string& ccpu_version, const std::string& wers_country_code, const std::string& language) { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SetSystemInfo(ccpu_version, wers_country_code, language); + CallPolicyManagerFunction( + &PolicyManager::SetSystemInfo, ccpu_version, wers_country_code, language); } void PolicyHandler::OnSystemInfoUpdateRequired() { @@ -961,8 +957,8 @@ void PolicyHandler::OnVehicleDataUpdated( return; } if (message[strings::msg_params].keyExists(strings::vin)) { - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SetVINValue( + CallPolicyManagerFunction( + &PolicyManager::SetVINValue, message[strings::msg_params][strings::vin].asString()); } #else @@ -999,8 +995,8 @@ void PolicyHandler::OnPendingPermissionChange( mobile_apis::AudioStreamingState::NOT_AUDIBLE, mobile_apis::VideoStreamingState::NOT_STREAMABLE, true); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->RemovePendingPermissionChanges(policy_app_id); + CallPolicyManagerFunction(&PolicyManager::RemovePendingPermissionChanges, + policy_app_id); return; } @@ -1049,7 +1045,7 @@ void PolicyHandler::OnPendingPermissionChange( } CallPolicyManagerFunction(&PolicyManager::RemovePendingPermissionChanges, - policy_app_id); + policy_app_id); } bool PolicyHandler::SendMessageToSDK(const BinaryMessage& pt_string, @@ -1145,7 +1141,7 @@ struct SDLAlowedNotification { void operator()(const ApplicationSharedPtr& app) { DCHECK_OR_RETURN_VOID(policy_manager_); - if (app->device() == device_id_) { + if (app && (app->device() == device_id_)) { std::string hmi_level; mobile_apis::HMILevel::eType default_mobile_hmi; { @@ -1291,8 +1287,7 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( void PolicyHandler::OnIgnitionCycleOver() { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->IncrementIgnitionCycles(); + CallPolicyManagerFunction(&PolicyManager::IncrementIgnitionCycles); } void PolicyHandler::OnActivateApp(uint32_t connection_key, @@ -1356,8 +1351,8 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, #else // EXTERNAL_PROPRIETARY_MODE permissions.isSDLAllowed = true; #endif // EXTERNAL_PROPRIETARY_MODE - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->RemovePendingPermissionChanges(policy_app_id); + CallPolicyManagerFunction(&PolicyManager::RemovePendingPermissionChanges, + policy_app_id); } // If application is revoked it should not be activated // In this case we need to activate application @@ -1378,8 +1373,7 @@ void PolicyHandler::KmsChanged(int kilometers) { POLICY_LIB_CHECK_VOID(); LOG4CXX_DEBUG(logger_, "PolicyHandler::KmsChanged " << kilometers << " kilometers"); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->KmsChanged(kilometers); + CallPolicyManagerFunction(&PolicyManager::KmsChanged, kilometers); } void PolicyHandler::PTExchangeAtUserRequest(uint32_t correlation_id) { @@ -1545,8 +1539,8 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string) { bool PolicyHandler::GetPriority(const std::string& policy_app_id, std::string* priority) const { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetPriority(policy_app_id, priority); + return CallPolicyManagerFunction( + &PolicyManager::GetPriority, policy_app_id, priority); } void PolicyHandler::CheckPermissions( @@ -1571,44 +1565,53 @@ void PolicyHandler::CheckPermissions( << hmi_level << " on device " << device_id << " rpc " << rpc); - sync_primitives::AutoLock lock(policy_manager_lock_); #ifdef EXTERNAL_PROPRIETARY_MODE - policy_manager_->CheckPermissions( - app->policy_app_id(), hmi_level, rpc, rpc_params, result); + CallPolicyManagerFunction(&PolicyManager::CheckPermissions, + app->policy_app_id(), + hmi_level, + rpc, + rpc_params, + result); #else // EXTERNAL_PROPRIETARY_MODE - policy_manager_->CheckPermissions( - device_id, app->policy_app_id(), hmi_level, rpc, rpc_params, result); + CallPolicyManagerFunction(&PolicyManager::CheckPermissions, + device_id, + app->policy_app_id(), + hmi_level, + rpc, + rpc_params, + result); #endif // EXTERNAL_PROPRIETARY_MODE } uint32_t PolicyHandler::GetNotificationsNumber( const std::string& priority) const { POLICY_LIB_CHECK(0); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetNotificationsNumber(priority); + return CallPolicyManagerFunction(&PolicyManager::GetNotificationsNumber, + priority); } DeviceConsent PolicyHandler::GetUserConsentForDevice( const std::string& device_id) const { POLICY_LIB_CHECK(kDeviceDisallowed); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetUserConsentForDevice(device_id); + return CallPolicyManagerFunction(&PolicyManager::GetUserConsentForDevice, + device_id); } bool PolicyHandler::GetDefaultHmi(const std::string& policy_app_id, std::string* default_hmi) const { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetDefaultHmi(policy_app_id, default_hmi); + return CallPolicyManagerFunction( + &PolicyManager::GetDefaultHmi, policy_app_id, default_hmi); } bool PolicyHandler::GetInitialAppData(const std::string& application_id, StringArray* nicknames, StringArray* app_hmi_types) { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetInitialAppData( - application_id, nicknames, app_hmi_types); + return CallPolicyManagerFunction(&PolicyManager::GetInitialAppData, + application_id, + nicknames, + app_hmi_types); } void PolicyHandler::GetUpdateUrls(const std::string& service_type, @@ -1627,14 +1630,12 @@ void PolicyHandler::GetUpdateUrls(const uint32_t service_type, std::string PolicyHandler::GetLockScreenIconUrl() const { POLICY_LIB_CHECK(std::string("")); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetLockScreenIconUrl(); + return CallPolicyManagerFunction(&PolicyManager::GetLockScreenIconUrl); } uint32_t PolicyHandler::NextRetryTimeout() { POLICY_LIB_CHECK(0); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->NextRetryTimeout(); + return CallPolicyManagerFunction(&PolicyManager::NextRetryTimeout); } uint32_t PolicyHandler::TimeoutExchangeSec() const { @@ -1643,26 +1644,22 @@ uint32_t PolicyHandler::TimeoutExchangeSec() const { uint32_t PolicyHandler::TimeoutExchangeMSec() const { POLICY_LIB_CHECK(0); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->TimeoutExchangeMSec(); + return CallPolicyManagerFunction(&PolicyManager::TimeoutExchangeMSec); } void PolicyHandler::OnExceededTimeout() { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->OnExceededTimeout(); + CallPolicyManagerFunction(&PolicyManager::OnExceededTimeout); } void PolicyHandler::OnSystemReady() { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->OnSystemReady(); + CallPolicyManagerFunction(&PolicyManager::OnSystemReady); } void PolicyHandler::PTUpdatedAt(Counters counter, int value) { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->PTUpdatedAt(counter, value); + CallPolicyManagerFunction(&PolicyManager::PTUpdatedAt, counter, value); } void PolicyHandler::add_listener(PolicyHandlerObserver* listener) { @@ -1864,26 +1861,25 @@ void PolicyHandler::RemoveDevice(const std::string& device_id) { bool PolicyHandler::IsApplicationRevoked(const std::string& app_id) { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->IsApplicationRevoked(app_id); + return CallPolicyManagerFunction(&PolicyManager::IsApplicationRevoked, + app_id); } void PolicyHandler::OnUpdateRequestSentToMobile() { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->OnUpdateStarted(); + CallPolicyManagerFunction(&PolicyManager::OnUpdateStarted); } bool PolicyHandler::CheckKeepContext(const std::string& policy_app_id) const { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->CanAppKeepContext(policy_app_id); + return CallPolicyManagerFunction(&PolicyManager::CanAppKeepContext, + policy_app_id); } bool PolicyHandler::CheckStealFocus(const std::string& policy_app_id) const { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->CanAppStealFocus(policy_app_id); + return CallPolicyManagerFunction(&PolicyManager::CanAppStealFocus, + policy_app_id); } bool PolicyHandler::CheckSystemAction( @@ -1907,8 +1903,7 @@ bool PolicyHandler::CheckSystemAction( uint32_t PolicyHandler::HeartBeatTimeout(const std::string& app_id) const { POLICY_LIB_CHECK(0); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->HeartBeatTimeout(app_id); + return CallPolicyManagerFunction(&PolicyManager::HeartBeatTimeout, app_id); } const std::string PolicyHandler::RemoteAppsUrl() const { @@ -1926,34 +1921,32 @@ const std::string PolicyHandler::RemoteAppsUrl() const { void PolicyHandler::OnAppsSearchStarted() { POLICY_LIB_CHECK(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->OnAppsSearchStarted(); + CallPolicyManagerFunction(&PolicyManager::OnAppsSearchStarted); } void PolicyHandler::OnAppsSearchCompleted(const bool trigger_ptu) { POLICY_LIB_CHECK(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->OnAppsSearchCompleted(trigger_ptu); + CallPolicyManagerFunction(&PolicyManager::OnAppsSearchCompleted, trigger_ptu); } void PolicyHandler::OnAppRegisteredOnMobile(const std::string& application_id) { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->OnAppRegisteredOnMobile(application_id); + CallPolicyManagerFunction(&PolicyManager::OnAppRegisteredOnMobile, + application_id); } RequestType::State PolicyHandler::GetAppRequestTypeState( const std::string& policy_app_id) const { POLICY_LIB_CHECK(RequestType::State::UNAVAILABLE); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetAppRequestTypesState(policy_app_id); + return CallPolicyManagerFunction(&PolicyManager::GetAppRequestTypesState, + policy_app_id); } RequestSubType::State PolicyHandler::GetAppRequestSubTypeState( const std::string& policy_app_id) const { POLICY_LIB_CHECK(RequestSubType::State::UNAVAILABLE); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetAppRequestSubTypesState(policy_app_id); + return CallPolicyManagerFunction(&PolicyManager::GetAppRequestSubTypesState, + policy_app_id); } bool PolicyHandler::IsRequestTypeAllowed( @@ -2036,8 +2029,8 @@ bool PolicyHandler::IsRequestSubTypeAllowed( const std::vector PolicyHandler::GetAppRequestTypes( const std::string& policy_app_id) const { POLICY_LIB_CHECK(std::vector()); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetAppRequestTypes(policy_app_id); + return CallPolicyManagerFunction(&PolicyManager::GetAppRequestTypes, + policy_app_id); } const std::vector PolicyHandler::GetAppRequestSubTypes( @@ -2048,15 +2041,13 @@ const std::vector PolicyHandler::GetAppRequestSubTypes( const VehicleInfo policy::PolicyHandler::GetVehicleInfo() const { POLICY_LIB_CHECK(VehicleInfo()); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetVehicleInfo(); + return CallPolicyManagerFunction(&PolicyManager::GetVehicleInfo); } #ifdef EXTERNAL_PROPRIETARY_MODE const MetaInfo PolicyHandler::GetMetaInfo() const { POLICY_LIB_CHECK(MetaInfo()); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetMetaInfo(); + return CallPolicyManagerFunction(&PolicyManager::GetMetaInfo); } #endif // EXTERNAL_PROPRIETARY_MODE @@ -2077,16 +2068,15 @@ void PolicyHandler::Set(const std::string& app_id, usage_statistics::AppInfoId type, const std::string& value) { POLICY_LIB_CHECK(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->Set(app_id, type, value); + CallPolicyManagerFunction(&PolicyManager::Set, app_id, type, value); } void PolicyHandler::Add(const std::string& app_id, usage_statistics::AppStopwatchId type, int32_t timespan_seconds) { POLICY_LIB_CHECK(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->Add(app_id, type, timespan_seconds); + CallPolicyManagerFunction( + &PolicyManager::Add, app_id, type, timespan_seconds); } bool PolicyHandler::IsUrlAppIdValid(const uint32_t app_idx, @@ -2132,15 +2122,14 @@ void PolicyHandler::UpdateHMILevel(ApplicationSharedPtr app, bool PolicyHandler::CheckModule(const PTString& app_id, const PTString& module) { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->CheckModule(app_id, module); + return CallPolicyManagerFunction(&PolicyManager::CheckModule, app_id, module); } void PolicyHandler::OnRemoteAppPermissionsChanged( const std::string& device_id, const std::string& application_id) { POLICY_LIB_CHECK_VOID(); - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SendAppPermissionsChanged(device_id, application_id); + CallPolicyManagerFunction( + &PolicyManager::SendAppPermissionsChanged, device_id, application_id); } void PolicyHandler::OnUpdateHMIStatus(const std::string& device_id, @@ -2175,8 +2164,8 @@ void PolicyHandler::OnUpdateHMIStatus(const std::string& device_id, bool PolicyHandler::GetModuleTypes(const std::string& policy_app_id, std::vector* modules) const { POLICY_LIB_CHECK(false); - sync_primitives::AutoLock lock(policy_manager_lock_); - return policy_manager_->GetModuleTypes(policy_app_id, modules); + return CallPolicyManagerFunction( + &PolicyManager::GetModuleTypes, policy_app_id, modules); } void PolicyHandler::SetDefaultHmiTypes( @@ -2192,8 +2181,8 @@ void PolicyHandler::SetDefaultHmiTypes( SmartObjectToInt()); } - sync_primitives::AutoLock lock(policy_manager_lock_); - policy_manager_->SetDefaultHmiTypes(application_id, hmi_types); + CallPolicyManagerFunction( + &PolicyManager::SetDefaultHmiTypes, application_id, hmi_types); } bool PolicyHandler::CheckHMIType(const std::string& application_id, @@ -2241,12 +2230,4 @@ void PolicyHandler::OnUpdateHMILevel(const std::string& device_id, UpdateHMILevel(app, level); } -template -auto PolicyHandler::CallPolicyManagerFunction(Func func, Args... args) const - -> decltype((std::declval().* - std::declval())(std::declval()...)) { - sync_primitives::AutoLock lock(policy_manager_lock_); - return ((*policy_manager_).*func)(args...); -} - } // namespace policy From 98dc17dda0378940c016a6b9a16f1e3992ee8e65 Mon Sep 17 00:00:00 2001 From: sniukalov Date: Fri, 9 Nov 2018 14:45:36 +0200 Subject: [PATCH 4/4] Apply const. --- .../src/policies/policy_handler.cc | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 2d6b220e560..8b890cc510c 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -879,7 +879,7 @@ void PolicyHandler::OnDeviceSwitching(const std::string& device_id_from, void PolicyHandler::OnGetStatusUpdate(const uint32_t correlation_id) { POLICY_LIB_CHECK_VOID(); - std::string policy_table_status = + const std::string policy_table_status = CallPolicyManagerFunction(&PolicyManager::GetPolicyTableStatus); MessageHelper::SendGetStatusUpdateResponse( @@ -981,7 +981,7 @@ void PolicyHandler::OnPendingPermissionChange( return; } - AppPermissions permissions = CallPolicyManagerFunction( + const AppPermissions permissions = CallPolicyManagerFunction( &PolicyManager::GetAppPermissionsChanges, policy_app_id); const uint32_t app_id = app->app_id(); @@ -1086,7 +1086,8 @@ bool PolicyHandler::ReceiveMessageFromSDK(const std::string& file, const BinaryMessage& pt_string) { POLICY_LIB_CHECK(false); - bool ret = CallPolicyManagerFunction(&PolicyManager::LoadPT, file, pt_string); + const bool ret = + CallPolicyManagerFunction(&PolicyManager::LoadPT, file, pt_string); LOG4CXX_INFO(logger_, "Policy table is saved: " << std::boolalpha << ret); if (ret) { @@ -1320,11 +1321,13 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, application_manager_.connection_handler().get_session_observer()); permissions.deviceInfo = device_params; - DeviceConsent consent = - CallPolicyManagerFunction(&PolicyManager::GetUserConsentForDevice, - permissions.deviceInfo.device_mac_address); + { + DeviceConsent consent = + CallPolicyManagerFunction(&PolicyManager::GetUserConsentForDevice, + permissions.deviceInfo.device_mac_address); - permissions.isSDLAllowed = kDeviceAllowed == consent; + permissions.isSDLAllowed = kDeviceAllowed == consent; + } // According to the SDLAQ-CRS-2794, p.9 // 'priority' should be omitted in case when device @@ -1380,7 +1383,7 @@ void PolicyHandler::PTExchangeAtUserRequest(uint32_t correlation_id) { POLICY_LIB_CHECK_VOID(); LOG4CXX_DEBUG(logger_, "PT exchange at user request"); - std::string update_status = + const std::string update_status = CallPolicyManagerFunction(&PolicyManager::ForcePTExchangeAtUserRequest); MessageHelper::SendUpdateSDLResponse( @@ -2191,7 +2194,7 @@ bool PolicyHandler::CheckHMIType(const std::string& application_id, POLICY_LIB_CHECK(false); std::vector policy_hmi_types; - bool ret = CallPolicyManagerFunction( + const bool ret = CallPolicyManagerFunction( &PolicyManager::GetHMITypes, application_id, &policy_hmi_types); std::vector additional_hmi_types;