Remove locks from settings

Settings should be initialized on start-up.  This removes the option of
lazily loading the settings from file and instead requires all settings
needed to be loaded on KiFACE start before requesting data from the
settings object

eeschema/CMakeLists.txt

@@ -33,6 +33,7 @@ include_directories(
     ${CMAKE_SOURCE_DIR}/common
     ${CMAKE_SOURCE_DIR}/common/dialogs
     ${CMAKE_SOURCE_DIR}/libs/sexpr/include
+    ${CMAKE_SOURCE_DIR}/3d-viewer
     ${INC_AFTER}
     ./dialogs
     ./libview

eeschema/eeschema.cpp

@@ -61,6 +61,9 @@
 #include <panel_sym_display_options.h>
 #include <sim/simulator_frame.h>
 
+#include "../3d-viewer/3d_viewer/eda_3d_viewer_settings.h"
+#include "../pcbnew/pcbnew_settings.h"
+
 #include <wx/crt.h>
 
 // The main sheet of the project
@@ -337,6 +340,11 @@ bool IFACE::OnKifaceStart( PGM_BASE* aProgram, int aCtlBits )
     InitSettings( new EESCHEMA_SETTINGS );
     aProgram->GetSettingsManager().RegisterSettings( KifaceSettings() );
 
+    // Register the symbol editor settings as well because they share a KiFACE and need to be
+    // loaded prior to use to avoid threading deadlocks
+    aProgram->GetSettingsManager().RegisterSettings( new SYMBOL_EDITOR_SETTINGS, false );
+    aProgram->GetSettingsManager().RegisterSettings( new SYMBOL_EDITOR_SETTINGS, false );
+
     start_common( aCtlBits );
 
     if( !loadGlobalLibTable() )

eeschema/eeschema_settings.cpp

@@ -774,7 +774,7 @@ bool EESCHEMA_SETTINGS::MigrateFromLegacy( wxConfigBase* aCfg )
     // LibEdit settings were stored with eeschema.  If eeschema is the first app to run,
     // we need to migrate the LibEdit settings here
 
-    auto libedit = Pgm().GetSettingsManager().GetAppSettings<SYMBOL_EDITOR_SETTINGS>( false );
+    auto libedit = Pgm().GetSettingsManager().GetAppSettings<SYMBOL_EDITOR_SETTINGS>();
     libedit->MigrateFromLegacy( aCfg );
     libedit->Load();
 

include/settings/settings_manager.h

@@ -23,7 +23,6 @@
 
 #include <algorithm>
 #include <mutex>
-#include <shared_mutex>
 #include <typeinfo>
 #include <core/wx_stl_compat.h> // for wxString hash
 #include <settings/color_settings.h>
@@ -68,11 +67,7 @@ public:
     template<typename T>
     T* RegisterSettings( T* aSettings, bool aLoadNow = true )
     {
-        // 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 ) );
+        return static_cast<T*>( registerSettings( aSettings, aLoadNow ) );
     }
 
     void Load();
@@ -100,32 +95,17 @@ public:
      * @return a pointer to a loaded settings object
      */
     template<typename T>
-    T* GetAppSettings( bool aLoadNow = true )
+    T* GetAppSettings()
     {
         T*     ret      = nullptr;
         size_t typeHash = typeid( T ).hash_code();
 
-        std::shared_lock<std::shared_mutex> readLock( m_settings_mutex );
-
          if( m_app_settings_cache.count( typeHash ) )
             ret = dynamic_cast<T*>( m_app_settings_cache.at( typeHash ) );
 
         if( 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(),
                                 []( const std::unique_ptr<JSON_SETTINGS>& aSettings )
                                 {
@@ -138,13 +118,7 @@ public:
         }
         else
         {
-            try
-            {
-                ret = static_cast<T*>( registerSettings( new T, aLoadNow ) );
-            }
-            catch( ... )
-            {
-            }
+            throw std::runtime_error( "Tried to GetAppSettings before registering" );
         }
 
         m_app_settings_cache[typeHash] = ret;
@@ -475,9 +449,6 @@ private:
     /// Lock for loaded project (expand to multiple once we support MDI)
     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;
 };
 

pcbnew/pcbnew.cpp

@@ -37,6 +37,7 @@
 #include <pcbnew_settings.h>
 #include <footprint_editor_settings.h>
 #include <settings/settings_manager.h>
+#include <settings/cvpcb_settings.h>
 #include <fp_lib_table.h>
 #include <footprint_edit_frame.h>
 #include <footprint_viewer_frame.h>
@@ -349,6 +350,12 @@ bool IFACE::OnKifaceStart( PGM_BASE* aProgram, int aCtlBits )
     InitSettings( new PCBNEW_SETTINGS );
     aProgram->GetSettingsManager().RegisterSettings( KifaceSettings() );
 
+    // Register the footprint editor settings as well because they share a KiFACE and need to be
+    // loaded prior to use to avoid threading deadlocks
+    aProgram->GetSettingsManager().RegisterSettings( new FOOTPRINT_EDITOR_SETTINGS );
+    aProgram->GetSettingsManager().RegisterSettings( new CVPCB_SETTINGS );
+    aProgram->GetSettingsManager().RegisterSettings( new EDA_3D_VIEWER_SETTINGS );
+
     start_common( aCtlBits );
 
     if( !loadGlobalLibTable() )
@@ -505,4 +512,4 @@ void IFACE::SaveFileAs( const wxString& aProjectBasePath, const wxString& aSrcPr
 int IFACE::HandleJob( JOB* aJob )
 {
     return m_jobHandler->RunJob( aJob );
-}
+}

pcbnew/pcbnew_settings.cpp

@@ -911,12 +911,12 @@ bool PCBNEW_SETTINGS::MigrateFromLegacy( wxConfigBase* aCfg )
     }
 
     // Footprint editor settings were stored in pcbnew config file.  Migrate them here.
-    auto fpedit = Pgm().GetSettingsManager().GetAppSettings<FOOTPRINT_EDITOR_SETTINGS>( false );
+    auto fpedit = Pgm().GetSettingsManager().GetAppSettings<FOOTPRINT_EDITOR_SETTINGS>();
     fpedit->MigrateFromLegacy( aCfg );
     fpedit->Load();
 
     // Same with 3D viewer
-    auto viewer3d = Pgm().GetSettingsManager().GetAppSettings<EDA_3D_VIEWER_SETTINGS>( false );
+    auto viewer3d = Pgm().GetSettingsManager().GetAppSettings<EDA_3D_VIEWER_SETTINGS>();
     viewer3d->MigrateFromLegacy( aCfg );
     viewer3d->Load();