Increased mutex safety.

Don't even query the size() without having at least a shared_lock.

*May* prevent KICAD-4S, but seems unlikely.
This commit is contained in:
Jeff Young 2023-06-05 10:01:05 +01:00
parent b41606ecf7
commit f3d3ade1dc
4 changed files with 32 additions and 14 deletions

View File

@ -932,7 +932,7 @@ bool PANEL_SYM_LIB_TABLE::TransferDataFromWindow()
m_globalTable->Clear(); m_globalTable->Clear();
m_globalTable->m_rows.transfer( m_globalTable->m_rows.end(), global_model()->m_rows.begin(), m_globalTable->m_rows.transfer( m_globalTable->m_rows.end(), global_model()->m_rows.begin(),
global_model()->m_rows.end(), global_model()->m_rows ); global_model()->m_rows.end(), global_model()->m_rows );
m_globalTable->reindex( true ); m_globalTable->reindex();
} }
if( project_model() && *project_model() != *m_projectTable ) if( project_model() && *project_model() != *m_projectTable )
@ -942,7 +942,7 @@ bool PANEL_SYM_LIB_TABLE::TransferDataFromWindow()
m_projectTable->Clear(); m_projectTable->Clear();
m_projectTable->m_rows.transfer( m_projectTable->m_rows.end(), project_model()->m_rows.begin(), m_projectTable->m_rows.transfer( m_projectTable->m_rows.end(), project_model()->m_rows.begin(),
project_model()->m_rows.end(), project_model()->m_rows ); project_model()->m_rows.end(), project_model()->m_rows );
m_projectTable->reindex( true ); m_projectTable->reindex();
} }
return true; return true;

View File

@ -462,7 +462,7 @@ public:
if( *iter == *aRow ) if( *iter == *aRow )
{ {
m_rows.erase( iter, iter + 1 ); m_rows.erase( iter, iter + 1 );
reindex( true ); reindex();
return true; return true;
} }
} }
@ -528,6 +528,18 @@ public:
return m_version; return m_version;
} }
/**
* While this is an encapsulation leak, calling it before threaded loads *may* prevent
* some Sentry crashes we're seeing (KICAD-4S).
*/
void EnsureIndex()
{
ensureIndex();
if( m_fallBack )
m_fallBack->EnsureIndex();
}
protected: protected:
/** /**
* Return a #LIB_TABLE_ROW if \a aNickname is found in this table or in any chained * Return a #LIB_TABLE_ROW if \a aNickname is found in this table or in any chained
@ -553,16 +565,10 @@ protected:
* @param aForce is to avoid rebuilding the index multiple times because multiple threads hit ensureIndex * @param aForce is to avoid rebuilding the index multiple times because multiple threads hit ensureIndex
* at the same time * at the same time
*/ */
void reindex( bool aForce ) void reindex()
{ {
std::lock_guard<std::shared_mutex> lock( m_nickIndexMutex ); std::lock_guard<std::shared_mutex> lock( m_nickIndexMutex );
if( !aForce )
{
if( m_nickIndex.size() )
return;
}
m_nickIndex.clear(); m_nickIndex.clear();
for( LIB_TABLE_ROWS_ITER it = m_rows.begin(); it != m_rows.end(); ++it ) for( LIB_TABLE_ROWS_ITER it = m_rows.begin(); it != m_rows.end(); ++it )
@ -574,8 +580,14 @@ protected:
// The dialog lib table editor may not maintain the nickIndex. // The dialog lib table editor may not maintain the nickIndex.
// Lazy indexing may be required. To handle lazy indexing, we must enforce // Lazy indexing may be required. To handle lazy indexing, we must enforce
// that "nickIndex" is either empty or accurate, but never inaccurate. // that "nickIndex" is either empty or accurate, but never inaccurate.
if( !m_nickIndex.size() ) {
reindex( false ); std::shared_lock<std::shared_mutex> lock( m_nickIndexMutex );
if( m_nickIndex.size() )
return;
}
reindex();
} }
private: private:

View File

@ -971,7 +971,7 @@ bool PANEL_FP_LIB_TABLE::TransferDataFromWindow()
m_global->Clear(); m_global->Clear();
m_global->m_rows.transfer( m_global->m_rows.end(), global_model()->m_rows.begin(), m_global->m_rows.transfer( m_global->m_rows.end(), global_model()->m_rows.begin(),
global_model()->m_rows.end(), global_model()->m_rows ); global_model()->m_rows.end(), global_model()->m_rows );
m_global->reindex( true ); m_global->reindex();
} }
if( project_model() && *project_model() != *m_project ) if( project_model() && *project_model() != *m_project )
@ -981,7 +981,7 @@ bool PANEL_FP_LIB_TABLE::TransferDataFromWindow()
m_project->Clear(); m_project->Clear();
m_project->m_rows.transfer( m_project->m_rows.end(), project_model()->m_rows.begin(), m_project->m_rows.transfer( m_project->m_rows.end(), project_model()->m_rows.begin(),
project_model()->m_rows.end(), project_model()->m_rows ); project_model()->m_rows.end(), project_model()->m_rows );
m_project->reindex( true ); m_project->reindex();
} }
return true; return true;

View File

@ -268,6 +268,12 @@ void FOOTPRINT_LIST_IMPL::loadFootprints()
return 1; return 1;
}; };
// While the private ensureIndex() is supposedly thread-safe, and having a public call is
// a bit of an encapsulation leak, we have at least one non-reproducible Sentry issue
// (KICAD-4S) that *might* be prevented by doing the EnsureIndex() before spooling up the
// multi-threaded part.
m_lib_table->EnsureIndex();
for( size_t ii = 0; ii < num_elements; ++ii ) for( size_t ii = 0; ii < num_elements; ++ii )
returns[ii] = tp.submit( fp_thread ); returns[ii] = tp.submit( fp_thread );