haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use read/write locks in the settings_manager to avoid thread issues in some scenarios 

Fixes sentry issue KICAD-90


(cherry picked from commit ab28650d57)
This commit is contained in:
Marek Roszko 2023-03-25 10:44:34 -04:00 committed by Mark Roszko
parent f2ce9cf857
commit 42b7aa6450
1 changed files with 26 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
include/settings/settings_manager.h
View File

@ -22,6 +22,7 @@
#define _SETTINGS_MANAGER_H #define _SETTINGS_MANAGER_H
#include <algorithm> #include <algorithm>
#include <shared_mutex>
#include <typeinfo> #include <typeinfo>
#include <core/wx_stl_compat.h> // for wxString hash #include <core/wx_stl_compat.h> // for wxString hash
#include <settings/color_settings.h> #include <settings/color_settings.h>
@ -66,7 +67,11 @@ public:
template<typename T> template<typename T>
T* RegisterSettings( T* aSettings, bool aLoadNow = true ) T* RegisterSettings( T* aSettings, bool aLoadNow = true )
{ {
return static_cast<T*>( registerSettings( aSettings, aLoadNow ) ); // Throws a low chance of conflict with GetAppSettings in threads but let's lock anyway
std::unique_lock<std::shared_mutex> writeLock( m_settings_mutex );
return static_cast<T*>(
registerSettings( aSettings, aLoadNow ) );
} }
void Load(); void Load();
@ -99,12 +104,27 @@ public:
T* ret = nullptr; T* ret = nullptr;
size_t typeHash = typeid( T ).hash_code(); size_t typeHash = typeid( T ).hash_code();
std::shared_lock<std::shared_mutex> readLock( m_settings_mutex );
if( m_app_settings_cache.count( typeHash ) ) if( m_app_settings_cache.count( typeHash ) )
ret = dynamic_cast<T*>( m_app_settings_cache.at( typeHash ) ); ret = dynamic_cast<T*>( m_app_settings_cache.at( typeHash ) );
if( ret ) if( ret )
return ret; return ret;
// cache lookup failed, now we shift to an exclusive lock to update the cache
readLock.unlock();
std::unique_lock<std::shared_mutex> writeLock( m_settings_mutex );
// In case multiple threads ended up waiting to unique lock to handle the same setting object
// This happens with library loading for example
if( m_app_settings_cache.count( typeHash ) )
{
T* recheck = dynamic_cast<T*>( m_app_settings_cache.at( typeHash ) );
if( recheck )
return recheck;
}
auto it = std::find_if( m_settings.begin(), m_settings.end(), auto it = std::find_if( m_settings.begin(), m_settings.end(),
[]( const std::unique_ptr<JSON_SETTINGS>& aSettings ) []( const std::unique_ptr<JSON_SETTINGS>& aSettings )
{ {
@ -119,12 +139,11 @@ public:
{ {
try try
{ {
ret = static_cast<T*>( RegisterSettings( new T, aLoadNow ) ); ret = static_cast<T*>( registerSettings( new T, aLoadNow ) );
} }
catch( ... ) catch( ... )
{ {
} }
} }
m_app_settings_cache[typeHash] = ret; m_app_settings_cache[typeHash] = ret;
@ -353,9 +372,9 @@ public:
static std::string GetSettingsVersion(); static std::string GetSettingsVersion();
private: private:
JSON_SETTINGS* registerSettings( JSON_SETTINGS* aSettings, bool aLoadNow = true ); JSON_SETTINGS* registerSettings( JSON_SETTINGS* aSettings, bool aLoadNow = true );
/** /**
* Determines the base path for user settings files. * Determines the base path for user settings files.
* *
@ -455,6 +474,9 @@ private:
/// Lock for loaded project (expand to multiple once we support MDI) /// Lock for loaded project (expand to multiple once we support MDI)
std::unique_ptr<wxSingleInstanceChecker> m_project_lock; std::unique_ptr<wxSingleInstanceChecker> m_project_lock;
/// Mutex to protect read/write access to m_settings and m_app_settings_cache
std::shared_mutex m_settings_mutex;
static wxString backupDateTimeFormat; static wxString backupDateTimeFormat;
}; };