From 995b2c517df71673b64f3a07de8f4904986927ba Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Fri, 15 Jul 2022 16:37:33 -0700 Subject: [PATCH] Catch some crashes on shutdown These can happen when a tool is active that sends signals when exiting (e.g. deselectEvent). These may be caught by the active loop in another tool which might try to update the UI after it has been freed. By marking all tools as "shutdown", the only event returned to them should be null. As an extra precaution, we flag the shutdown globally within the tool manager and check this flag before launching either events or new tools Fixes https://gitlab.com/kicad/code/kicad/issues/10698 (cherry picked from commit db4f2d9dd828c861963ae3e08d83d06ad12adf1b) --- common/tool/tool_manager.cpp | 23 ++++++++++++++++++++++- include/tool/tool_manager.h | 3 +++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/common/tool/tool_manager.cpp b/common/tool/tool_manager.cpp index 680d71de07..40236d40ab 100644 --- a/common/tool/tool_manager.cpp +++ b/common/tool/tool_manager.cpp @@ -206,7 +206,8 @@ TOOL_MANAGER::TOOL_MANAGER() : m_warpMouseAfterContextMenu( true ), m_menuActive( false ), m_menuOwner( -1 ), - m_activeState( nullptr ) + m_activeState( nullptr ), + m_shuttingDown( false ) { m_actionMgr = new ACTION_MANAGER( this ); } @@ -313,6 +314,9 @@ VECTOR2D TOOL_MANAGER::GetCursorPosition() const bool TOOL_MANAGER::RunAction( const TOOL_ACTION& aAction, bool aNow, void* aParam ) { + if( m_shuttingDown ) + return true; + bool handled = false; TOOL_EVENT event = aAction.MakeEvent(); @@ -446,10 +450,22 @@ bool TOOL_MANAGER::runTool( TOOL_BASE* aTool ) void TOOL_MANAGER::ShutdownAllTools() { + m_shuttingDown = true; + // Create a temporary list of tools to iterate over since when the tools shutdown // they remove themselves from the list automatically (invalidating the iterator) ID_LIST tmpList = m_activeTools; + // Make sure each tool knows that it is shutting down, so that loops get shut down + // at the dispatcher + for( auto id : tmpList ) + { + if( m_toolIdIndex.count( id ) == 0 ) + continue; + + m_toolIdIndex[id]->shutdown = true; + } + for( auto id : tmpList ) { ShutdownTool( id ); @@ -949,6 +965,11 @@ TOOL_MANAGER::ID_LIST::iterator TOOL_MANAGER::finishTool( TOOL_STATE* aState ) bool TOOL_MANAGER::ProcessEvent( const TOOL_EVENT& aEvent ) { + // Once the tool manager is shutting down, don't start + // activating more tools + if( m_shuttingDown ) + return true; + bool handled = processEvent( aEvent ); TOOL_STATE* activeTool = GetCurrentToolState(); diff --git a/include/tool/tool_manager.h b/include/tool/tool_manager.h index 40943fa97e..18a5146c55 100644 --- a/include/tool/tool_manager.h +++ b/include/tool/tool_manager.h @@ -578,6 +578,9 @@ private: ///< Pointer to the state object corresponding to the currently executed tool. TOOL_STATE* m_activeState; + + ///< True if the tool manager is shutting down (don't process additional events) + bool m_shuttingDown; }; #endif // __TOOL_MANAGER_H