Implement thread safety for symbol loading routines

These functions can be called from any thread of the library
loader, so we need to ensure we have some thread safety with them.

common/lib_table_base.cpp

@@ -185,6 +185,8 @@ LIB_TABLE_ROW* LIB_TABLE::findRow( const wxString& aNickName, bool aCheckIfEnabl
     LIB_TABLE_ROW* row = nullptr;
     LIB_TABLE* cur = (LIB_TABLE*) this;
 
+    std::lock_guard<std::recursive_mutex> lock( m_nickIndexMutex );
+
     do
     {
         cur->ensureIndex();
@@ -301,6 +303,8 @@ std::vector<wxString> LIB_TABLE::GetLogicalLibs()
 
 bool LIB_TABLE::InsertRow( LIB_TABLE_ROW* aRow, bool doReplace )
 {
+    std::lock_guard<std::recursive_mutex> lock( m_nickIndexMutex );
+
     ensureIndex();
 
     INDEX_CITER it = nickIndex.find( aRow->GetNickName() );

common/template_fieldnames.cpp

@@ -22,6 +22,8 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
  */
 
+#include <mutex>
+
 #include <macros.h>
 #include <template_fieldnames.h>
 #include <pgm_base.h>
@@ -33,6 +35,8 @@ using namespace TFIELD_T;
 #define FTPCANONICAL "Footprint"
 #define DSHCANONICAL "Datasheet"
 
+static std::mutex s_defaultFieldMutex;
+
 const wxString TEMPLATE_FIELDNAME::GetDefaultFieldName( int aFieldNdx, bool aTranslate )
 {
     static void* locale = nullptr;
@@ -53,6 +57,10 @@ const wxString TEMPLATE_FIELDNAME::GetDefaultFieldName( int aFieldNdx, bool aTra
         }
     }
 
+    // Mutex protection is needed so that multiple loader threads don't write to the static variables
+    // at once
+    std::lock_guard<std::mutex> lock( s_defaultFieldMutex );
+
     // Fetching translations can take a surprising amount of time when loading libraries,
     // so only do it when necessary.
     if( Pgm().GetLocale() != locale )

eeschema/class_library.cpp

@@ -56,7 +56,7 @@
 PART_LIB::PART_LIB( SCH_LIB_TYPE aType, const wxString& aFileName, SCH_IO_MGR::SCH_FILE_T aPluginType ) :
     // start @ != 0 so each additional library added
     // is immediately detectable, zero would not be.
-    m_mod_hash( PART_LIBS::s_modify_generation ),
+    m_mod_hash( PART_LIBS::GetModifyGeneration() ),
     m_pluginType( aPluginType )
 {
     type = aType;
@@ -413,7 +413,8 @@ void PART_LIBS::FindLibraryNearEntries( std::vector<LIB_PART*>& aCandidates,
 }
 
 
-int PART_LIBS::s_modify_generation = 1;     // starts at 1 and goes up
+int        PART_LIBS::s_modify_generation = 1;     // starts at 1 and goes up
+std::mutex PART_LIBS::s_generationMutex;
 
 
 int PART_LIBS::GetModifyHash()
@@ -428,7 +429,7 @@ int PART_LIBS::GetModifyHash()
     // Rebuilding the cache (m_cache) does not change the GetModHash() value,
     // but changes PART_LIBS::s_modify_generation.
     // Take this change in account:
-    hash += PART_LIBS::s_modify_generation;
+    hash += PART_LIBS::GetModifyGeneration();
 
     return hash;
 }

eeschema/class_library.h

@@ -32,6 +32,7 @@
 #define CLASS_LIBRARY_H
 
 #include <map>
+#include <mutex>
 #include <boost/ptr_container/ptr_vector.hpp>
 #include <wx/filename.h>
 
@@ -197,11 +198,24 @@ class PART_LIBS : public PART_LIBS_BASE, public PROJECT::_ELEM
 public:
     KICAD_T Type() override { return PART_LIBS_T; }
 
-    static int s_modify_generation;         ///< helper for GetModifyHash()
+    static int        s_modify_generation;         ///< helper for GetModifyHash()
+    static std::mutex s_generationMutex;
 
     PART_LIBS()
     {
-        ++s_modify_generation;
+        IncrementModifyGeneration();
+    }
+
+    static void IncrementModifyGeneration()
+    {
+        std::lock_guard<std::mutex> mut( PART_LIBS::s_generationMutex );
+        ++PART_LIBS::s_modify_generation;
+    }
+
+    static int GetModifyGeneration()
+    {
+        std::lock_guard<std::mutex> mut( PART_LIBS::s_generationMutex );
+        return PART_LIBS::s_modify_generation;
     }
 
     /// Return the modification hash for all libraries.  The value returned

eeschema/eeschema_config.cpp

@@ -21,6 +21,8 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
  */
 
+#include <mutex>
+
 #include <class_library.h>
 #include <confirm.h>
 #include <dialogs/panel_eeschema_color_settings.h>
@@ -224,8 +226,14 @@ void SYMBOL_EDIT_FRAME::InstallPreferences( PAGED_DIALOG* aParent,
 }
 
 
+static std::mutex s_symbolTableMutex;
+
+
 SYMBOL_LIB_TABLE* PROJECT::SchSymbolLibTable()
 {
+    wxLogDebug( "Getting symbol lib table" );
+    std::lock_guard<std::mutex> lock( s_symbolTableMutex );
+
     // This is a lazy loading function, it loads the project specific table when
     // that table is asked for, not before.
     SYMBOL_LIB_TABLE* tbl = (SYMBOL_LIB_TABLE*) GetElem( ELEM_SYMBOL_LIB_TABLE );

eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp

@@ -777,7 +777,7 @@ void SCH_SEXPR_PLUGIN::Format( SCH_SHEET* aSheet )
 }
 
 
-void SCH_SEXPR_PLUGIN::Format( EE_SELECTION* aSelection, SCH_SHEET_PATH* aSheetPath, 
+void SCH_SEXPR_PLUGIN::Format( EE_SELECTION* aSelection, SCH_SHEET_PATH* aSheetPath,
                                OUTPUTFORMATTER* aFormatter )
 {
     wxCHECK( aSelection && aFormatter, /* void */ );
@@ -2094,7 +2094,7 @@ void SCH_SEXPR_PLUGIN::cacheLib( const wxString& aLibraryFileName, const PROPERT
         // Because m_cache is rebuilt, increment PART_LIBS::s_modify_generation
         // to modify the hash value that indicate component to symbol links
         // must be updated.
-        PART_LIBS::s_modify_generation++;
+        PART_LIBS::IncrementModifyGeneration();
 
         if( !isBuffering( aProperties ) )
             m_cache->Load();

eeschema/sch_plugins/legacy/sch_legacy_plugin.cpp

@@ -23,6 +23,7 @@
 #include <algorithm>
 #include <boost/algorithm/string/join.hpp>
 #include <cctype>
+#include <mutex>
 #include <set>
 
 #include <wx/mstream.h>
@@ -476,8 +477,7 @@ static void parseQuotedString( wxString& aString, LINE_READER& aReader,
  */
 class SCH_LEGACY_PLUGIN_CACHE
 {
-    static int      m_modHash;      // Keep track of the modification status of the library.
-
+    static int      s_modHash;      // Keep track of the modification status of the library.
     wxString        m_fileName;     // Absolute path and file name.
     wxFileName      m_libFileName;  // Absolute path and file name is required here.
     wxDateTime      m_fileModTime;
@@ -523,11 +523,23 @@ class SCH_LEGACY_PLUGIN_CACHE
 
     friend SCH_LEGACY_PLUGIN;
 
+    static std::mutex s_modHashMutex;
+
 public:
     SCH_LEGACY_PLUGIN_CACHE( const wxString& aLibraryPath );
     ~SCH_LEGACY_PLUGIN_CACHE();
 
-    int GetModifyHash() const { return m_modHash; }
+    static void IncrementModifyHash()
+    {
+        std::lock_guard<std::mutex> mut( SCH_LEGACY_PLUGIN_CACHE::s_modHashMutex );
+        SCH_LEGACY_PLUGIN_CACHE::s_modHash++;
+    }
+
+    static int GetModifyHash()
+    {
+        std::lock_guard<std::mutex> mut( SCH_LEGACY_PLUGIN_CACHE::s_modHashMutex );
+        return SCH_LEGACY_PLUGIN_CACHE::s_modHash;
+    }
 
     // Most all functions in this class throw IO_ERROR exceptions.  There are no
     // error codes nor user interface calls from here, nor in any SCH_PLUGIN objects.
@@ -2395,7 +2407,8 @@ void SCH_LEGACY_PLUGIN::saveBusAlias( std::shared_ptr<BUS_ALIAS> aAlias )
 }
 
 
-int SCH_LEGACY_PLUGIN_CACHE::m_modHash = 1;     // starts at 1 and goes up
+int SCH_LEGACY_PLUGIN_CACHE::s_modHash = 1;     // starts at 1 and goes up
+std::mutex SCH_LEGACY_PLUGIN_CACHE::s_modHashMutex;
 
 
 SCH_LEGACY_PLUGIN_CACHE::SCH_LEGACY_PLUGIN_CACHE( const wxString& aFullPathAndFileName ) :
@@ -2533,7 +2546,7 @@ LIB_PART* SCH_LEGACY_PLUGIN_CACHE::removeSymbol( LIB_PART* aPart )
     m_symbols.erase( it );
     delete aPart;
     m_isModified = true;
-    ++m_modHash;
+    SCH_LEGACY_PLUGIN_CACHE::IncrementModifyHash();
     return firstChild;
 }
 
@@ -2551,7 +2564,7 @@ void SCH_LEGACY_PLUGIN_CACHE::AddSymbol( const LIB_PART* aPart )
 
     m_symbols[ name ] = const_cast< LIB_PART* >( aPart );
     m_isModified = true;
-    ++m_modHash;
+    SCH_LEGACY_PLUGIN_CACHE::IncrementModifyHash();
 }
 
 
@@ -2631,7 +2644,7 @@ void SCH_LEGACY_PLUGIN_CACHE::Load()
         }
     }
 
-    ++m_modHash;
+    SCH_LEGACY_PLUGIN_CACHE::IncrementModifyHash();
 
     // Remember the file modification time of library file when the
     // cache snapshot was made, so that in a networked environment we will
@@ -4199,7 +4212,7 @@ void SCH_LEGACY_PLUGIN_CACHE::DeleteSymbol( const wxString& aSymbolName )
         delete part;
     }
 
-    ++m_modHash;
+    SCH_LEGACY_PLUGIN_CACHE::IncrementModifyHash();
     m_isModified = true;
 }
 
@@ -4215,7 +4228,7 @@ void SCH_LEGACY_PLUGIN::cacheLib( const wxString& aLibraryFileName, const PROPER
         // Because m_cache is rebuilt, increment PART_LIBS::s_modify_generation
         // to modify the hash value that indicate component to symbol links
         // must be updated.
-        PART_LIBS::s_modify_generation++;
+        PART_LIBS::IncrementModifyGeneration();
 
         if( !isBuffering( aProperties ) )
             m_cache->Load();
@@ -4243,7 +4256,7 @@ bool SCH_LEGACY_PLUGIN::isBuffering( const PROPERTIES* aProperties )
 int SCH_LEGACY_PLUGIN::GetModifyHash() const
 {
     if( m_cache )
-        return m_cache->GetModifyHash();
+        return SCH_LEGACY_PLUGIN_CACHE::GetModifyHash();
 
     // If the cache hasn't been loaded, it hasn't been modified.
     return 0;

include/lib_table_base.h

@@ -30,6 +30,7 @@
 #include <boost/noncopyable.hpp>
 #include <boost/ptr_container/ptr_vector.hpp>
 #include <memory>
+#include <mutex>
 #include <project.h>
 #include <properties.h>
 #include <richio.h>
@@ -318,6 +319,8 @@ public:
     /// Delete all rows.
     void Clear()
     {
+        std::lock_guard<std::recursive_mutex> lock( m_nickIndexMutex );
+
         rows.clear();
         nickIndex.clear();
     }
@@ -501,6 +504,8 @@ protected:
 
     void reindex()
     {
+        std::lock_guard<std::recursive_mutex> lock( m_nickIndexMutex );
+
         nickIndex.clear();
 
         for( LIB_TABLE_ROWS_ITER it = rows.begin(); it != rows.end(); ++it )
@@ -509,6 +514,8 @@ protected:
 
     void ensureIndex()
     {
+        std::lock_guard<std::recursive_mutex> lock( m_nickIndexMutex );
+
         // The dialog lib table editor may not maintain the nickIndex.
         // Lazy indexing may be required.  To handle lazy indexing, we must enforce
         // that "nickIndex" is either empty or accurate, but never inaccurate.
@@ -533,6 +540,9 @@ protected:
     INDEX nickIndex;
 
     LIB_TABLE* fallBack;
+
+    /// Mutex to protect access to the nickIndex variable
+    mutable std::recursive_mutex m_nickIndexMutex;
 };
 
 #endif  // _LIB_TABLE_BASE_H_