From ed6440b782f5293af25c48208ff0f51dbf3299ca Mon Sep 17 00:00:00 2001 From: Tomasz Wlostowski Date: Mon, 23 Jun 2025 00:49:56 +0200 Subject: [PATCH] router: clean up shoved traces after aborting differential pair routing in shove mode Note: the single-trace placer (LINE_PLACER) has moved to the FIXED_TAIL for tracking the interim placed routes (and reverting them if user pressed backspace). Unfortunately the DIFF_PAIR_PLACER didn't follow. The result was the dummy CommitPlacement() call at the end of aborted routing would commit the last shove state, even if FixRoute() was never called. For longer-term fix: fix ROUTER_TOOL logic to indicated aborted routing to the router/placer (AbortPlacement() is already there). Also implement FIXED_TAIL in diff pair placement mode. --- pcbnew/router/pns_diff_pair_placer.cpp | 13 +++++++++++-- pcbnew/router/pns_diff_pair_placer.h | 2 ++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pcbnew/router/pns_diff_pair_placer.cpp b/pcbnew/router/pns_diff_pair_placer.cpp index 4ba811cf8f..9bbb3ea936 100644 --- a/pcbnew/router/pns_diff_pair_placer.cpp +++ b/pcbnew/router/pns_diff_pair_placer.cpp @@ -642,6 +642,7 @@ bool DIFF_PAIR_PLACER::Start( const VECTOR2I& aP, ITEM* aStartItem ) m_currentTraceOk = false; m_currentTrace = DIFF_PAIR(); m_currentTrace.SetNets( m_netP, m_netN ); + m_lastFixNode = nullptr; initPlacement(); @@ -842,9 +843,15 @@ bool DIFF_PAIR_PLACER::FixRoute( const VECTOR2I& aP, ITEM* aEndItem, bool aForce topo.SimplifyLine( &lineN ); m_prevPair = m_currentTrace.EndingPrimitives(); + m_lastFixNode = m_lastNode; + + // avoid an use-after-free error (CommitPlacement calls NODE::Commit which will invalidate the shove heads state. Need to rethink the memory management). + if( Settings().Mode() == RM_Shove ) + m_shove = std::make_unique( m_world, Router() ); CommitPlacement(); m_placingVia = false; + m_lastFixNode = nullptr; if( m_snapOnTarget || aForceFinish ) { @@ -862,6 +869,7 @@ bool DIFF_PAIR_PLACER::FixRoute( const VECTOR2I& aP, ITEM* aEndItem, bool aForce bool DIFF_PAIR_PLACER::AbortPlacement() { m_world->KillChildren(); + m_lastNode = nullptr; return true; } @@ -874,9 +882,10 @@ bool DIFF_PAIR_PLACER::HasPlacedAnything() const bool DIFF_PAIR_PLACER::CommitPlacement() { - if( m_lastNode ) - Router()->CommitRouting( m_lastNode ); + if( m_lastFixNode ) + Router()->CommitRouting( m_lastFixNode ); + m_lastFixNode = nullptr; m_lastNode = nullptr; m_currentNode = nullptr; return true; diff --git a/pcbnew/router/pns_diff_pair_placer.h b/pcbnew/router/pns_diff_pair_placer.h index 7fdc7a8c08..ae691ef842 100644 --- a/pcbnew/router/pns_diff_pair_placer.h +++ b/pcbnew/router/pns_diff_pair_placer.h @@ -249,6 +249,7 @@ private: ///< Postprocessed world state (including marked collisions & removed loops) NODE* m_lastNode; + NODE* m_lastFixNode; SIZES_SETTINGS m_sizes; @@ -277,6 +278,7 @@ private: ITEM* m_currentEndItem; bool m_idle; + bool m_hasFixedAnything; }; }