From 06e58a92960d7590657df599be554bf94eda39db Mon Sep 17 00:00:00 2001 From: Andrii Kalinich Date: Tue, 22 Oct 2019 13:33:27 -0400 Subject: Synchronize backup and reset policy functions This is an issue specific for EXTERNAL_PROPRIETARY flow only. This issue is exposed by PolicyHandlerTest unit test: ResetPolicyTable_WithPreloadedFile_ExpectPolicyTableReset which sometimes is failing. The main problem was that database reset(drop) and backup can be done simultaneously in different threads which were not synchronized. In that case, database drop often may be failed because another thread is trying to write something into the DB table which is going to be dropped. This causes above unit test to fail sometimes. To make that unit test and policy component more stable, these two threads were synchronized by additing a waiting function, so one thread will be able to wait for another thread to finish its own staff. --- .../policy_external/include/policy/cache_manager.h | 17 +++++++++++++++ .../policy/policy_external/src/cache_manager.cc | 25 ++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/components/policy/policy_external/include/policy/cache_manager.h b/src/components/policy/policy_external/include/policy/cache_manager.h index acc7a7da51..5873d31781 100644 --- a/src/components/policy/policy_external/include/policy/cache_manager.h +++ b/src/components/policy/policy_external/include/policy/cache_manager.h @@ -33,6 +33,7 @@ #ifndef SRC_COMPONENTS_POLICY_POLICY_EXTERNAL_INCLUDE_POLICY_CACHE_MANAGER_H_ #define SRC_COMPONENTS_POLICY_POLICY_EXTERNAL_INCLUDE_POLICY_CACHE_MANAGER_H_ +#include #include #include "boost/optional.hpp" @@ -1107,12 +1108,28 @@ class CacheManager : public CacheManagerInterface { ~BackgroundBackuper(); virtual void threadMain(); virtual void exitThreadMain(); + + /** + * @brief Notifies BackgroundBackuper thread that new data is available and + * new backup iteration should be scheduled + */ void DoBackup(); + /** + * @brief Waits for BackgroundBackuper thread finish its own backup + * iteration. If currently no backup iteration in progress - function just + * returns the control back + */ + void WaitForBackupIsDone(); + private: void InternalBackup(); CacheManager* cache_manager_; sync_primitives::ConditionalVariable backup_notifier_; + sync_primitives::ConditionalVariable backup_done_; + sync_primitives::Lock backup_done_lock_; + + std::atomic_bool backup_is_in_progress_; volatile bool stop_flag_; volatile bool new_data_available_; diff --git a/src/components/policy/policy_external/src/cache_manager.cc b/src/components/policy/policy_external/src/cache_manager.cc index 3e1538ab04..1c5b5bff6c 100644 --- a/src/components/policy/policy_external/src/cache_manager.cc +++ b/src/components/policy/policy_external/src/cache_manager.cc @@ -2682,10 +2682,14 @@ bool CacheManager::LoadFromFile(const std::string& file_name, bool CacheManager::ResetPT(const std::string& file_name) { LOG4CXX_AUTO_TRACE(logger_); is_unpaired_.clear(); + + backuper_->WaitForBackupIsDone(); + if (!backup_->RefreshDB()) { LOG4CXX_ERROR(logger_, "Can't re-create policy database. Reset failed."); return false; } + sync_primitives::AutoLock lock(cache_lock_); pt_.reset(new policy_table::Table()); const bool result = LoadFromFile(file_name, *pt_); @@ -3097,6 +3101,7 @@ void CacheManager::OnDeviceSwitching(const std::string& device_id_from, CacheManager::BackgroundBackuper::BackgroundBackuper( CacheManager* cache_manager) : cache_manager_(cache_manager) + , backup_is_in_progress_(false) , stop_flag_(false) , new_data_available_(false) { LOG4CXX_AUTO_TRACE(logger_); @@ -3121,13 +3126,23 @@ void CacheManager::BackgroundBackuper::threadMain() { LOG4CXX_AUTO_TRACE(logger_); sync_primitives::AutoLock lock(need_backup_lock_); while (!stop_flag_) { + backup_is_in_progress_.exchange(true); { sync_primitives::AutoUnlock need_backup_lock(need_backup_lock_); InternalBackup(); } + if (new_data_available_ || stop_flag_) { continue; } + + { + LOG4CXX_DEBUG(logger_, "Backup is done"); + sync_primitives::AutoLock auto_lock(backup_done_lock_); + backup_is_in_progress_.exchange(false); + backup_done_.Broadcast(); + } + LOG4CXX_DEBUG(logger_, "Wait for a next backup"); backup_notifier_.Wait(need_backup_lock_); } @@ -3147,6 +3162,16 @@ void CacheManager::BackgroundBackuper::DoBackup() { backup_notifier_.NotifyOne(); } +void CacheManager::BackgroundBackuper::WaitForBackupIsDone() { + LOG4CXX_AUTO_TRACE(logger_); + sync_primitives::AutoLock auto_lock(backup_done_lock_); + if (!backup_is_in_progress_) { + return; + } + + backup_done_.Wait(auto_lock); +} + EncryptionRequired CacheManager::GetAppEncryptionRequiredFlag( const std::string& application) const { LOG4CXX_AUTO_TRACE(logger_); -- cgit v1.2.1 From 32d697135fa021d2cf45366d4ebf0ac9e941accd Mon Sep 17 00:00:00 2001 From: Andrii Kalinich Date: Thu, 24 Oct 2019 09:16:27 -0400 Subject: fixup! Synchronize backup and reset policy functions --- src/components/policy/policy_external/src/cache_manager.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/components/policy/policy_external/src/cache_manager.cc b/src/components/policy/policy_external/src/cache_manager.cc index 1c5b5bff6c..18b663469e 100644 --- a/src/components/policy/policy_external/src/cache_manager.cc +++ b/src/components/policy/policy_external/src/cache_manager.cc @@ -3132,10 +3132,6 @@ void CacheManager::BackgroundBackuper::threadMain() { InternalBackup(); } - if (new_data_available_ || stop_flag_) { - continue; - } - { LOG4CXX_DEBUG(logger_, "Backup is done"); sync_primitives::AutoLock auto_lock(backup_done_lock_); @@ -3143,6 +3139,10 @@ void CacheManager::BackgroundBackuper::threadMain() { backup_done_.Broadcast(); } + if (new_data_available_ || stop_flag_) { + continue; + } + LOG4CXX_DEBUG(logger_, "Wait for a next backup"); backup_notifier_.Wait(need_backup_lock_); } @@ -3165,11 +3165,9 @@ void CacheManager::BackgroundBackuper::DoBackup() { void CacheManager::BackgroundBackuper::WaitForBackupIsDone() { LOG4CXX_AUTO_TRACE(logger_); sync_primitives::AutoLock auto_lock(backup_done_lock_); - if (!backup_is_in_progress_) { - return; + if (backup_is_in_progress_) { + backup_done_.Wait(auto_lock); } - - backup_done_.Wait(auto_lock); } EncryptionRequired CacheManager::GetAppEncryptionRequiredFlag( -- cgit v1.2.1