From 3d526edc8a0190a5a8f465946de741a5f1c3aac3 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@craftyjon.com>
Date: Sun, 9 Feb 2025 09:18:03 -0500
Subject: [PATCH] More thread safety for TRACKS_CLEANER
Fixes https://gitlab.com/kicad/code/kicad/-/issues/19884
---
pcbnew/connectivity/connectivity_algo.cpp | 14 +++----
.../dialog_cleanup_tracks_and_vias.cpp | 38 +++++++++++++------
2 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/pcbnew/connectivity/connectivity_algo.cpp b/pcbnew/connectivity/connectivity_algo.cpp
index a96e9ff5e5..1bb9b6c512 100644
--- a/pcbnew/connectivity/connectivity_algo.cpp
+++ b/pcbnew/connectivity/connectivity_algo.cpp
@@ -350,8 +350,10 @@ CN_CONNECTIVITY_ALGO::SearchClusters( CLUSTER_SEARCH_MODE aMode, const std::vect
if( m_itemList.IsDirty() )
searchConnections();
+ std::set<CN_ITEM*> visited;
+
auto addToSearchList =
- [&item_set, withinAnyNet, aSingleNet, &aTypes, rootItem ]( CN_ITEM *aItem )
+ [&item_set, withinAnyNet, aSingleNet, &aTypes, rootItem, &visited]( CN_ITEM *aItem )
{
if( withinAnyNet && aItem->Net() <= 0 )
return;
@@ -376,8 +378,6 @@ CN_CONNECTIVITY_ALGO::SearchClusters( CLUSTER_SEARCH_MODE aMode, const std::vect
if( !found && aItem != rootItem )
return;
- aItem->SetVisited( false );
-
item_set.insert( aItem );
};
@@ -392,14 +392,14 @@ CN_CONNECTIVITY_ALGO::SearchClusters( CLUSTER_SEARCH_MODE aMode, const std::vect
CN_ITEM* root;
auto it = item_set.begin();
- while( it != item_set.end() && (*it)->Visited() )
+ while( it != item_set.end() && visited.contains( *it ) )
it = item_set.erase( item_set.begin() );
if( it == item_set.end() )
break;
root = *it;
- root->SetVisited( true );
+ visited.insert( root );
Q.clear();
Q.push_back( root );
@@ -416,9 +416,9 @@ CN_CONNECTIVITY_ALGO::SearchClusters( CLUSTER_SEARCH_MODE aMode, const std::vect
if( withinAnyNet && n->Net() != root->Net() )
continue;
- if( !n->Visited() && n->Valid() )
+ if( !visited.contains( n ) && n->Valid() )
{
- n->SetVisited( true );
+ visited.insert( n );
Q.push_back( n );
}
}
diff --git a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp
index 63030da517..ae1fc7db76 100644
--- a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp
+++ b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp
@@ -188,10 +188,29 @@ void DIALOG_CLEANUP_TRACKS_AND_VIAS::doCleanup( bool aDryRun )
BOARD_COMMIT commit( m_parentFrame );
TRACKS_CLEANER cleaner( m_brd, commit );
+ struct FILTER_STATE
+ {
+ bool selectedOnly;
+ int netCodeOnly;
+ wxString netClassOnly;
+ int layerOnly;
+ };
+
+ FILTER_STATE filter_state = {
+ .selectedOnly = m_selectedItemsFilter->GetValue(),
+ .netCodeOnly = m_netFilterOpt->GetValue() ? m_netFilter->GetSelectedNetcode() : -1,
+ .netClassOnly = m_netclassFilterOpt->GetValue()
+ ? m_netclassFilter->GetStringSelection()
+ : wxString(),
+ .layerOnly = m_layerFilterOpt->GetValue()
+ ? m_layerFilter->GetLayerSelection()
+ : UNDEFINED_LAYER
+ };
+
cleaner.SetFilter(
- [&]( BOARD_CONNECTED_ITEM* aItem ) -> bool
+ [&, filter_state]( BOARD_CONNECTED_ITEM* aItem ) -> bool
{
- if( m_selectedItemsFilter->GetValue() )
+ if( filter_state.selectedOnly )
{
if( !aItem->IsSelected() )
{
@@ -205,26 +224,23 @@ void DIALOG_CLEANUP_TRACKS_AND_VIAS::doCleanup( bool aDryRun )
}
}
- if( m_netFilterOpt->GetValue() && m_netFilter->GetSelectedNetcode() >= 0 )
+ if( filter_state.netCodeOnly >= 0 )
{
- if( aItem->GetNetCode() != m_netFilter->GetSelectedNetcode() )
+ if( aItem->GetNetCode() != filter_state.netCodeOnly )
return true;
}
- if( m_netclassFilterOpt->GetValue()
- && !m_netclassFilter->GetStringSelection().IsEmpty() )
+ if( !filter_state.netClassOnly.IsEmpty() )
{
- wxString filterNetclass = m_netclassFilter->GetStringSelection();
NETCLASS* netclass = aItem->GetEffectiveNetClass();
- if( !netclass->ContainsNetclassWithName( filterNetclass ) )
+ if( !netclass->ContainsNetclassWithName( filter_state.netClassOnly ) )
return true;
}
- if( m_layerFilterOpt->GetValue()
- && m_layerFilter->GetLayerSelection() != UNDEFINED_LAYER )
+ if( filter_state.layerOnly != UNDEFINED_LAYER )
{
- if( aItem->GetLayer() != m_layerFilter->GetLayerSelection() )
+ if( aItem->GetLayer() != filter_state.layerOnly )
return true;
}