From b30f01258aeea4ec4c9cde22e942f091ade1cbfb Mon Sep 17 00:00:00 2001 From: RomanReznichenkoLuxoft <85177915+RomanReznichenkoLuxoft@users.noreply.github.com> Date: Tue, 29 Mar 2022 17:01:04 +0300 Subject: Add missed locks (#3890) Co-authored-by: OlhaVorobiova --- .../rpc_protection_manager_impl.h | 2 +- .../src/rpc_protection_manager_impl.cc | 18 ++++++++++-------- .../policy_regular/include/policy/cache_manager.h | 2 +- .../policy/policy_regular/src/access_remote_impl.cc | 4 +++- .../policy/policy_regular/src/cache_manager.cc | 21 +++++++++++---------- .../policy_regular/src/sql_pt_representation.cc | 1 + 6 files changed, 27 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/components/application_manager/include/application_manager/rpc_protection_manager_impl.h b/src/components/application_manager/include/application_manager/rpc_protection_manager_impl.h index b971ace480..f5e96e4420 100644 --- a/src/components/application_manager/include/application_manager/rpc_protection_manager_impl.h +++ b/src/components/application_manager/include/application_manager/rpc_protection_manager_impl.h @@ -106,7 +106,7 @@ class RPCProtectionManagerImpl : public RPCProtectionManager { policy::PolicyHandlerInterface& policy_handler_; AppEncryptedRpcMap encrypted_rpcs_; - sync_primitives::Lock encrypted_rpcs_lock_; + mutable sync_primitives::RecursiveLock encrypted_rpcs_lock_; std::set encryption_needed_cache_; sync_primitives::Lock message_needed_encryption_lock_; diff --git a/src/components/application_manager/src/rpc_protection_manager_impl.cc b/src/components/application_manager/src/rpc_protection_manager_impl.cc index 64b732ddb2..62e296e786 100644 --- a/src/components/application_manager/src/rpc_protection_manager_impl.cc +++ b/src/components/application_manager/src/rpc_protection_manager_impl.cc @@ -89,7 +89,7 @@ bool RPCProtectionManagerImpl::CheckPolicyEncryptionFlag( bool RPCProtectionManagerImpl::IsEncryptionRequiredByPolicy( const std::string& policy_app_id, const std::string& function_name) const { SDL_LOG_AUTO_TRACE(); - + sync_primitives::AutoLock lock(encrypted_rpcs_lock_); auto it = encrypted_rpcs_.find(policy_app_id); if (encrypted_rpcs_.end() == it) { @@ -160,23 +160,25 @@ void RPCProtectionManagerImpl::OnPTUFinished(const bool ptu_result) { void RPCProtectionManagerImpl::SaveEncryptedRPC() { SDL_LOG_AUTO_TRACE(); - + sync_primitives::AutoLock lock(encrypted_rpcs_lock_); const auto policy_encryption_flag_getter = policy_handler_.PolicyEncryptionFlagGetter(); - const auto policy_policy_app_ids = - policy_encryption_flag_getter->GetApplicationPolicyIDs(); + if (policy_encryption_flag_getter) { + const auto policy_policy_app_ids = + policy_encryption_flag_getter->GetApplicationPolicyIDs(); - for (const auto& app : policy_policy_app_ids) { - SDL_LOG_DEBUG("Processing app name: " << app); + for (const auto& app : policy_policy_app_ids) { + SDL_LOG_DEBUG("Processing app name: " << app); - encrypted_rpcs_[app] = GetEncryptedRPCsForApp(app); + encrypted_rpcs_[app] = GetEncryptedRPCsForApp(app); + } } } void RPCProtectionManagerImpl::OnPTInited() { SDL_LOG_AUTO_TRACE(); - + sync_primitives::AutoLock lock(encrypted_rpcs_lock_); encrypted_rpcs_.clear(); SaveEncryptedRPC(); diff --git a/src/components/policy/policy_regular/include/policy/cache_manager.h b/src/components/policy/policy_regular/include/policy/cache_manager.h index 473caef145..2f19da13b5 100644 --- a/src/components/policy/policy_regular/include/policy/cache_manager.h +++ b/src/components/policy/policy_regular/include/policy/cache_manager.h @@ -988,8 +988,8 @@ class CacheManager : public CacheManagerInterface { BackgroundBackuper* backuper_; const PolicySettings* settings_; -#ifdef BUILD_TESTS friend class AccessRemoteImpl; +#ifdef BUILD_TESTS FRIEND_TEST(AccessRemoteImplTest, CheckModuleType); FRIEND_TEST(AccessRemoteImplTest, EnableDisable); FRIEND_TEST(AccessRemoteImplTest, GetGroups); diff --git a/src/components/policy/policy_regular/src/access_remote_impl.cc b/src/components/policy/policy_regular/src/access_remote_impl.cc index 439fbcc8d2..04bf9479ef 100644 --- a/src/components/policy/policy_regular/src/access_remote_impl.cc +++ b/src/components/policy/policy_regular/src/access_remote_impl.cc @@ -97,7 +97,7 @@ bool AccessRemoteImpl::CheckModuleType(const PTString& app_id, if (!cache_->IsApplicationRepresented(app_id)) { return false; } - + sync_primitives::AutoLock auto_lock(cache_->cache_lock_); const policy_table::ApplicationParams& app = cache_->pt()->policy_table.app_policies_section.apps[app_id]; if (!app.moduleType.is_initialized()) { @@ -168,6 +168,7 @@ void AccessRemoteImpl::SetDefaultHmiTypes(const ApplicationOnDevice& who, const policy_table::AppHMITypes& AccessRemoteImpl::HmiTypes( const ApplicationOnDevice& who) { SDL_LOG_AUTO_TRACE(); + sync_primitives::AutoLock auto_lock(cache_->cache_lock_); if (cache_->IsDefaultPolicy(who.app_id)) { return hmi_types_[who]; } else { @@ -226,6 +227,7 @@ void AccessRemoteImpl::GetGroupsIds(const std::string& device_id, bool AccessRemoteImpl::GetModuleTypes(const std::string& application_id, std::vector* modules) { DCHECK(modules); + sync_primitives::AutoLock auto_lock(cache_->cache_lock_); policy_table::ApplicationPolicies& apps = cache_->pt()->policy_table.app_policies_section.apps; policy_table::ApplicationPolicies::iterator i = apps.find(application_id); diff --git a/src/components/policy/policy_regular/src/cache_manager.cc b/src/components/policy/policy_regular/src/cache_manager.cc index 4d066e9c32..f22cd9eec0 100644 --- a/src/components/policy/policy_regular/src/cache_manager.cc +++ b/src/components/policy/policy_regular/src/cache_manager.cc @@ -220,7 +220,6 @@ void CacheManager::GetAllAppGroups(const std::string& app_id, SDL_LOG_INFO("Devices doesn't have groups"); return; } - sync_primitives::AutoLock auto_lock(cache_lock_); policy_table::ApplicationPolicies::const_iterator app_params_iter = pt_->policy_table.app_policies_section.apps.find(app_id); @@ -439,7 +438,6 @@ bool CacheManager::GetDeviceGroupsFromPolicies( bool CacheManager::AddDevice(const std::string& device_id, const std::string& connection_type) { SDL_LOG_AUTO_TRACE(); - sync_primitives::AutoLock auto_lock(cache_lock_); CACHE_MANAGER_CHECK(false); policy_table::DeviceParams& params = @@ -465,7 +463,6 @@ bool CacheManager::SetDeviceData(const std::string& device_id, const uint32_t number_of_ports, const std::string& connection_type) { SDL_LOG_AUTO_TRACE(); - sync_primitives::AutoLock auto_lock(cache_lock_); CACHE_MANAGER_CHECK(false); Backup(); @@ -1313,9 +1310,9 @@ void CacheManager::PersistData() { if (pt_.use_count() != 0) { // Comma expression is used to hold the lock only during the constructor // call + policy_table::Table copy_pt( (sync_primitives::AutoLock(cache_lock_), *pt_)); - backup_->Save(copy_pt); backup_->SaveUpdateRequired(update_required); @@ -1586,7 +1583,7 @@ void CacheManager::Increment(usage_statistics::GlobalCounterId type) { void CacheManager::Increment(const std::string& app_id, usage_statistics::AppCounterId type) { CACHE_MANAGER_CHECK_VOID(); - sync_primitives::AutoLock lock(cache_lock_); + sync_primitives::AutoLock auto_lock(cache_lock_); switch (type) { case usage_statistics::USER_SELECTIONS: ++(*pt_->policy_table.usage_and_error_counts->app_level)[app_id] @@ -1635,7 +1632,7 @@ void CacheManager::Set(const std::string& app_id, usage_statistics::AppInfoId type, const std::string& value) { CACHE_MANAGER_CHECK_VOID(); - sync_primitives::AutoLock lock(cache_lock_); + sync_primitives::AutoLock auto_lock(cache_lock_); switch (type) { case usage_statistics::LANGUAGE_GUI: (*pt_->policy_table.usage_and_error_counts->app_level)[app_id] @@ -1656,7 +1653,7 @@ void CacheManager::Add(const std::string& app_id, usage_statistics::AppStopwatchId type, int seconds) { CACHE_MANAGER_CHECK_VOID(); - sync_primitives::AutoLock lock(cache_lock_); + sync_primitives::AutoLock auto_lock(cache_lock_); const int minutes = ConvertSecondsToMinute(seconds); switch (type) { case usage_statistics::SECONDS_HMI_FULL: @@ -1778,6 +1775,7 @@ bool CacheManager::IsPredataPolicy(const std::string& app_id) const { bool CacheManager::SetUnpairedDevice(const std::string& device_id, bool unpaired) { + SDL_LOG_AUTO_TRACE(); sync_primitives::AutoLock auto_lock(cache_lock_); const bool result = pt_->policy_table.device_data->end() != pt_->policy_table.device_data->find(device_id); @@ -1805,6 +1803,7 @@ bool CacheManager::SetVINValue(const std::string& value) { } bool CacheManager::IsApplicationRepresented(const std::string& app_id) const { + SDL_LOG_AUTO_TRACE(); CACHE_MANAGER_CHECK(false); if (kDeviceId == app_id) { return true; @@ -1876,7 +1875,7 @@ void CacheManager::FillDeviceSpecificData() {} bool CacheManager::LoadFromBackup() { SDL_LOG_AUTO_TRACE(); - sync_primitives::AutoLock lock(cache_lock_); + sync_primitives::AutoLock auto_lock(cache_lock_); pt_ = backup_->GenerateSnapshot(); update_required = backup_->UpdateRequired(); SDL_LOG_DEBUG("Update required flag from backup: " << std::boolalpha @@ -1930,7 +1929,7 @@ bool CacheManager::LoadFromFile(const std::string& file_name, } SDL_LOG_TRACE("Start create PT"); - sync_primitives::AutoLock locker(cache_lock_); + sync_primitives::AutoLock auto_lock(cache_lock_); table = policy_table::Table(&value); @@ -1957,6 +1956,7 @@ bool CacheManager::ResetPT(const std::string& file_name) { } bool CacheManager::AppExists(const std::string& app_id) const { + SDL_LOG_AUTO_TRACE(); CACHE_MANAGER_CHECK(false); sync_primitives::AutoLock auto_lock(cache_lock_); policy_table::ApplicationPolicies::iterator policy_iter = @@ -2072,6 +2072,7 @@ void CacheManager::GetAppRequestSubTypes( } std::string CacheManager::GetCertificate() const { + SDL_LOG_AUTO_TRACE(); CACHE_MANAGER_CHECK(std::string("")); sync_primitives::AutoLock auto_lock(cache_lock_); if (pt_->policy_table.module_config.certificate.is_initialized()) { @@ -2088,7 +2089,7 @@ bool CacheManager::MergePreloadPT(const std::string& file_name) { return false; } - sync_primitives::AutoLock lock(cache_lock_); + sync_primitives::AutoLock auto_lock(cache_lock_); policy_table::PolicyTable& current = pt_->policy_table; policy_table::PolicyTable& new_table = table.policy_table; const std::string date_current = *current.module_config.preloaded_date; diff --git a/src/components/policy/policy_regular/src/sql_pt_representation.cc b/src/components/policy/policy_regular/src/sql_pt_representation.cc index 9dcde34588..f6ee8772fc 100644 --- a/src/components/policy/policy_regular/src/sql_pt_representation.cc +++ b/src/components/policy/policy_regular/src/sql_pt_representation.cc @@ -2676,6 +2676,7 @@ policy_table::VehicleDataItem SQLPTRepresentation::PopulateVDIFromQuery( bool SQLPTRepresentation::InsertVehicleDataItem( const policy_table::VehicleDataItem& vehicle_data_item) { + SDL_LOG_AUTO_TRACE(); utils::dbms::SQLQuery query(db()); if (!vehicle_data_item.is_initialized() || !vehicle_data_item.is_valid()) { -- cgit v1.2.1