Never call ReCreateMenuBar inside a menu event handler

As of wxWidgets 3.2, the wxWidgets event handler runs code after the
the client event handler that depends on the menu still existing.
Because there are potentially many paths to call ReCreateMenuBar from
within a menu event handler, let's just wrap this action in a CallAfter
to make sure it happens after the wx handler call completes.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/13149
This commit is contained in:
Jon Evans 2022-12-16 16:37:32 -05:00
parent 0e2a4e3f15
commit 5abf73e3c9
27 changed files with 54 additions and 32 deletions

View File

@ -206,7 +206,7 @@ wxWindow* BM2CMP_FRAME::GetToolCanvas() const
}
void BM2CMP_FRAME::ReCreateMenuBar()
void BM2CMP_FRAME::doReCreateMenuBar()
{
// wxWidgets handles the Mac Application menu behind the scenes, but that means
// we always have to start from scratch with a new wxMenuBar.

View File

@ -158,7 +158,8 @@ private:
wxWindow* GetToolCanvas() const override;
void ReCreateMenuBar() override;
protected:
void doReCreateMenuBar() override;
private:
wxImage m_Pict_Image;

View File

@ -445,6 +445,17 @@ void EDA_BASE_FRAME::setupUIConditions()
void EDA_BASE_FRAME::ReCreateMenuBar()
{
/**
* As of wxWidgets 3.2, recreating the menubar from within an event handler of that menubar
* will result in memory corruption on macOS. In order to minimize the chance of programmer
* error causing regressions here, we always wrap calls to ReCreateMenuBar in a CallAfter to
* ensure that they do not occur within the same event handling call stack.
*/
CallAfter( [=]()
{
doReCreateMenuBar();
} );
}

View File

@ -144,7 +144,6 @@ public:
* Functions to rebuild the toolbars and menubars
*/
void ReCreateHToolbar();
void ReCreateMenuBar() override;
void ShowChangedLanguage() override;
@ -304,6 +303,8 @@ public:
protected:
CVPCB_MAINFRAME( KIWAY* aKiway, wxWindow* aParent );
void doReCreateMenuBar() override;
void setupUIConditions() override;
bool canCloseWindow( wxCloseEvent& aCloseEvent ) override;

View File

@ -34,7 +34,7 @@
#include <widgets/wx_menubar.h>
void CVPCB_MAINFRAME::ReCreateMenuBar()
void CVPCB_MAINFRAME::doReCreateMenuBar()
{
COMMON_CONTROL* tool = m_toolManager->GetTool<COMMON_CONTROL>();
// wxWidgets handles the Mac Application menu behind the scenes, but that means

View File

@ -40,7 +40,7 @@
#include <advanced_config.h>
void SCH_EDIT_FRAME::ReCreateMenuBar()
void SCH_EDIT_FRAME::doReCreateMenuBar()
{
EE_SELECTION_TOOL* selTool = m_toolManager->GetTool<EE_SELECTION_TOOL>();
// wxWidgets handles the Mac Application menu behind the scenes, but that means

View File

@ -132,7 +132,6 @@ public:
void ReCreateHToolbar() override;
void ReCreateVToolbar() override;
void ReCreateOptToolbar() override;
void ReCreateMenuBar() override;
void setupUIConditions() override;
@ -837,6 +836,8 @@ protected:
*/
bool doAutoSave() override;
void doReCreateMenuBar() override;
/**
* Send the KiCad netlist over to CVPCB.
*/

View File

@ -34,7 +34,7 @@
#include <widgets/wx_menubar.h>
void SYMBOL_EDIT_FRAME::ReCreateMenuBar()
void SYMBOL_EDIT_FRAME::doReCreateMenuBar()
{
EE_SELECTION_TOOL* selTool = m_toolManager->GetTool<EE_SELECTION_TOOL>();
// wxWidgets handles the Mac Application menu behind the scenes, but that means

View File

@ -118,8 +118,6 @@ public:
SELECTION& GetCurrentSelection() override;
void ReCreateMenuBar() override;
// See comments for m_SyncPinEdit.
bool SynchronizePins();
@ -392,6 +390,8 @@ public:
protected:
void setupUIConditions() override;
void doReCreateMenuBar() override;
private:
// Set up the tool framework
void setupTools();

View File

@ -93,7 +93,6 @@ public:
void ReCreateHToolbar() override;
void ReCreateVToolbar() override;
void ReCreateOptToolbar() override {}
void ReCreateMenuBar() override;
void ClickOnLibList( wxCommandEvent& event );
void ClickOnSymbolList( wxCommandEvent& event );
@ -145,6 +144,8 @@ public:
protected:
void setupUIConditions() override;
void doReCreateMenuBar() override;
private:
// Set up the tool framework.
void setupTools();

View File

@ -97,7 +97,7 @@ void SYMBOL_VIEWER_FRAME::ReCreateVToolbar()
}
void SYMBOL_VIEWER_FRAME::ReCreateMenuBar()
void SYMBOL_VIEWER_FRAME::doReCreateMenuBar()
{
SYMBOL_EDITOR_CONTROL* libControl = m_toolManager->GetTool<SYMBOL_EDITOR_CONTROL>();
// wxWidgets handles the OSX Application menu behind the scenes, but that means

View File

@ -87,7 +87,6 @@ public:
*/
void ReCreateOptToolbar() override;
void ReCreateMenuBar() override;
void UpdateStatusBar() override;
void UpdateToolbarControlSizes() override;
@ -474,6 +473,7 @@ public:
protected:
void setupUIConditions() override;
void doReCreateMenuBar() override;
private:
void updateComponentListSelectBox();

View File

@ -38,7 +38,7 @@
#include <widgets/wx_menubar.h>
void GERBVIEW_FRAME::ReCreateMenuBar()
void GERBVIEW_FRAME::doReCreateMenuBar()
{
GERBVIEW_SELECTION_TOOL* selTool = m_toolManager->GetTool<GERBVIEW_SELECTION_TOOL>();
// wxWidgets handles the Mac Application menu behind the scenes, but that means

View File

@ -435,7 +435,7 @@ public:
*
* Needed when the language or icons are changed
*/
virtual void ReCreateMenuBar();
void ReCreateMenuBar();
/**
* Adds the standard KiCad help menu to the menubar.
@ -602,6 +602,8 @@ protected:
return wxT( "_autosave-" );
}
virtual void doReCreateMenuBar() {}
/**
* Handle the auto save timer event.
*/

View File

@ -195,7 +195,6 @@ public:
void EraseMsgBox();
void ReCreateMenuBar() override { }
virtual void ReCreateHToolbar() = 0;
virtual void ReCreateVToolbar() = 0;
virtual void ReCreateOptToolbar() = 0;
@ -460,6 +459,8 @@ protected:
void setupUnits( APP_SETTINGS_BASE* aCfg );
void doReCreateMenuBar() override { }
std::vector<wxWindow*> findDialogs();
/**

View File

@ -210,7 +210,6 @@ public:
// General
virtual void ReCreateOptToolbar() override { }
virtual void ShowChangedLanguage() override;
virtual void ReCreateMenuBar() override;
virtual void UpdateStatusBar() override;
PCB_SCREEN* GetScreen() const override { return (PCB_SCREEN*) EDA_DRAW_FRAME::GetScreen(); }
@ -384,6 +383,8 @@ protected:
void handleIconizeEvent( wxIconizeEvent& aEvent ) override;
virtual void doReCreateMenuBar() override;
/**
* Attempts to load \a aFootprintId from the footprint library table.
*

View File

@ -63,7 +63,6 @@ public:
void OnClearFileHistory( wxCommandEvent& aEvent );
void OnExit( wxCommandEvent& event );
void ReCreateMenuBar() override;
void RecreateBaseHToolbar();
wxString GetCurrentFileName() const override
@ -159,6 +158,8 @@ public:
protected:
virtual void setupUIConditions() override;
void doReCreateMenuBar() override;
private:
void setupTools();
void setupActions();

View File

@ -42,7 +42,7 @@
#include <wx/dir.h>
void KICAD_MANAGER_FRAME::ReCreateMenuBar()
void KICAD_MANAGER_FRAME::doReCreateMenuBar()
{
KICAD_MANAGER_CONTROL* controlTool = m_toolManager->GetTool<KICAD_MANAGER_CONTROL>();
// wxWidgets handles the Mac Application menu behind the scenes, but that means

View File

@ -38,7 +38,7 @@
#include "tools/pl_selection_tool.h"
void PL_EDITOR_FRAME::ReCreateMenuBar()
void PL_EDITOR_FRAME::doReCreateMenuBar()
{
PL_SELECTION_TOOL* selTool = m_toolManager->GetTool<PL_SELECTION_TOOL>();
// wxWidgets handles the Mac Application menu behind the scenes, but that means

View File

@ -142,8 +142,6 @@ public:
*/
void ReCreateOptToolbar() override;
void ReCreateMenuBar() override;
const PL_EDITOR_LAYOUT& GetPageLayout() const { return m_pageLayout; }
PL_EDITOR_LAYOUT& GetPageLayout() { return m_pageLayout; }
@ -276,6 +274,8 @@ protected:
void setupUIConditions() override;
void doReCreateMenuBar() override;
DECLARE_EVENT_TABLE();
/// The last filename chosen to be proposed to the user

View File

@ -117,11 +117,6 @@ public:
void ReCreateOptToolbar() override;
void UpdateToolbarControlSizes() override;
/**
* @brief (Re)Create the menubar for the Footprint Editor frame
*/
void ReCreateMenuBar() override;
/**
* Re create the layer Box by clearing the old list, and building a new one from the new
* layers names and layer colors..
@ -339,6 +334,11 @@ protected:
void restoreLastFootprint();
void retainLastFootprint();
/**
* @brief (Re)Create the menubar for the Footprint Editor frame
*/
void doReCreateMenuBar() override;
/**
* Run the Footprint Properties dialog and handle changes made in it.
*/

View File

@ -91,6 +91,8 @@ protected:
void setupUIConditions() override;
void doReCreateMenuBar() override;
private:
const wxString getCurNickname();
void setCurNickname( const wxString& aNickname );
@ -117,7 +119,6 @@ private:
void ReCreateHToolbar() override;
void ReCreateVToolbar() override;
void ReCreateOptToolbar() override;
void ReCreateMenuBar() override;
void OnLibFilter( wxCommandEvent& aEvent );
void OnFPFilter( wxCommandEvent& aEvent );

View File

@ -36,7 +36,7 @@
#include <widgets/wx_menubar.h>
void FOOTPRINT_EDIT_FRAME::ReCreateMenuBar()
void FOOTPRINT_EDIT_FRAME::doReCreateMenuBar()
{
PCB_SELECTION_TOOL* selTool = m_toolManager->GetTool<PCB_SELECTION_TOOL>();
// wxWidgets handles the Mac Application menu behind the scenes, but that means

View File

@ -40,7 +40,7 @@
#include <widgets/wx_menubar.h>
void PCB_EDIT_FRAME::ReCreateMenuBar()
void PCB_EDIT_FRAME::doReCreateMenuBar()
{
PCB_SELECTION_TOOL* selTool = m_toolManager->GetTool<PCB_SELECTION_TOOL>();
// wxWidgets handles the Mac Application menu behind the scenes, but that means

View File

@ -665,7 +665,7 @@ BOX2I PCB_BASE_FRAME::GetBoardBoundingBox( bool aBoardEdgesOnly ) const
// Virtual function
void PCB_BASE_FRAME::ReCreateMenuBar()
void PCB_BASE_FRAME::doReCreateMenuBar()
{
}

View File

@ -245,7 +245,6 @@ public:
void ReCreateAuxiliaryToolbar() override;
void ReCreateVToolbar() override;
void ReCreateOptToolbar() override;
void ReCreateMenuBar() override;
void UpdateToolbarControlSizes() override;
/**
@ -726,6 +725,8 @@ protected:
LAYER_TOOLBAR_ICON_VALUES m_prevIconVal;
void doReCreateMenuBar() override;
// The Tool Framework initialization
void setupTools();
void setupUIConditions() override;

View File

@ -159,7 +159,7 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateVToolbar()
}
void FOOTPRINT_VIEWER_FRAME::ReCreateMenuBar()
void FOOTPRINT_VIEWER_FRAME::doReCreateMenuBar()
{
PCB_SELECTION_TOOL* selTool = m_toolManager->GetTool<PCB_SELECTION_TOOL>();