Implement a more aggressive re-entrancy check for SCH drawing tools.

Fixes https://gitlab.com/kicad/code/kicad/issues/7259

common/tool/tools_holder.cpp

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2020 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2020-2021 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -23,7 +23,6 @@
 
 #include <pgm_base.h>
 #include <settings/common_settings.h>
-#include <tool/action_manager.h>
 #include <tool/action_menu.h>
 #include <tool/actions.h>
 #include <tool/tools_holder.h>
@@ -42,16 +41,8 @@ TOOLS_HOLDER::TOOLS_HOLDER() :
 
 // TODO: Implement an RAII mechanism for the stack PushTool/PopTool pairs
 void TOOLS_HOLDER::PushTool( const std::string& actionName )
-{
-    if( m_toolStack.size() && m_toolStack.back() == actionName )
-    {
-        // Tool already on the stack; we don't need to push again.  (Happens when one tool is
-        // popped due to the activation of the same tool that is underneath it on the stack.)
-    }
-    else
 {
     m_toolStack.push_back( actionName );
-    }
 
     // Human cognitive stacking is very shallow; deeper tool stacks just get annoying
     if( m_toolStack.size() > 3 )

eeschema/tools/sch_drawing_tools.cpp

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2019 CERN
- * Copyright (C) 2019-2020 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2019-2021 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -56,7 +56,12 @@ SCH_DRAWING_TOOLS::SCH_DRAWING_TOOLS() :
         m_lastGlobalLabelShape( PINSHEETLABEL_SHAPE::PS_INPUT ),
         m_lastTextOrientation( LABEL_SPIN_STYLE::LEFT ),
         m_lastTextBold( false ),
-        m_lastTextItalic( false )
+        m_lastTextItalic( false ),
+        m_inPlaceComponent( false ),
+        m_inPlaceImage( false ),
+        m_inSingleClickPlace( false ),
+        m_inTwoClickPlace( false ),
+        m_inDrawSheet( false )
 {
 }
 
@@ -84,6 +89,11 @@ int SCH_DRAWING_TOOLS::PlaceComponent(  const TOOL_EVENT& aEvent  )
     SCHLIB_FILTER               filter;
     std::vector<PICKED_SYMBOL>* historyList = nullptr;
 
+    if( m_inPlaceComponent )
+        return 0;
+    else
+        m_inPlaceComponent = true;
+
     if( aEvent.IsAction( &EE_ACTIONS::placeSymbol ) )
     {
         historyList = &m_symbolHistoryList;
@@ -305,6 +315,7 @@ int SCH_DRAWING_TOOLS::PlaceComponent(  const TOOL_EVENT& aEvent  )
     }
 
     m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ARROW );
+    m_inPlaceComponent = false;
     return 0;
 }
 
@@ -315,6 +326,11 @@ int SCH_DRAWING_TOOLS::PlaceImage( const TOOL_EVENT& aEvent )
     bool        immediateMode = image;
     VECTOR2I    cursorPos = getViewControls()->GetCursorPosition();
 
+    if( m_inPlaceImage )
+        return 0;
+    else
+        m_inPlaceImage = true;
+
     m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true );
     getViewControls()->ShowCursor( true );
 
@@ -479,6 +495,7 @@ int SCH_DRAWING_TOOLS::PlaceImage( const TOOL_EVENT& aEvent )
     }
 
     m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ARROW );
+    m_inPlaceImage = false;
     return 0;
 }
 
@@ -488,6 +505,11 @@ int SCH_DRAWING_TOOLS::SingleClickPlace( const TOOL_EVENT& aEvent )
     wxPoint   cursorPos;
     KICAD_T   type = aEvent.Parameter<KICAD_T>();
 
+    if( m_inSingleClickPlace )
+        return 0;
+    else
+        m_inSingleClickPlace = true;
+
     if( type == SCH_JUNCTION_T && aEvent.HasPosition() )
     {
         EE_SELECTION& selection = m_selectionTool->GetSelection();
@@ -689,6 +711,7 @@ int SCH_DRAWING_TOOLS::SingleClickPlace( const TOOL_EVENT& aEvent )
     m_view->ClearPreview();
 
     m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ARROW );
+    m_inSingleClickPlace = false;
     return 0;
 }
 
@@ -811,6 +834,11 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
     KIGFX::VIEW_CONTROLS* controls = getViewControls();
     EE_GRID_HELPER        grid( m_toolMgr );
 
+    if( m_inTwoClickPlace )
+        return 0;
+    else
+        m_inTwoClickPlace = true;
+
     bool      isImportMode  = aEvent.IsAction( &EE_ACTIONS::importSheetPin );
     bool      isText        = aEvent.IsAction( &EE_ACTIONS::placeSchematicText );
     bool      isGlobalLabel = aEvent.IsAction( &EE_ACTIONS::placeGlobalLabel );
@@ -1033,6 +1061,7 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent )
     }
 
     m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ARROW );
+    m_inTwoClickPlace = false;
     return 0;
 }
 
@@ -1041,6 +1070,11 @@ int SCH_DRAWING_TOOLS::DrawSheet( const TOOL_EVENT& aEvent )
 {
     SCH_SHEET* sheet = nullptr;
 
+    if( m_inDrawSheet )
+        return 0;
+    else
+        m_inDrawSheet = true;
+
     m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true );
     getViewControls()->ShowCursor( true );
 
@@ -1178,6 +1212,7 @@ int SCH_DRAWING_TOOLS::DrawSheet( const TOOL_EVENT& aEvent )
     }
 
     m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ARROW );
+    m_inDrawSheet = false;
     return 0;
 }
 

eeschema/tools/sch_drawing_tools.h

@@ -82,6 +82,13 @@ private:
     bool                       m_lastTextBold;
     bool                       m_lastTextItalic;
 
+    ///< Re-entrancy guards
+    bool                       m_inPlaceComponent;
+    bool                       m_inPlaceImage;
+    bool                       m_inSingleClickPlace;
+    bool                       m_inTwoClickPlace;
+    bool                       m_inDrawSheet;
+
     std::unique_ptr<STATUS_TEXT_POPUP> m_statusPopup;
 };