From c974a0d8889e28b380905d5d1503a43b0d1c8c0f Mon Sep 17 00:00:00 2001 From: Jonathan Li Date: Sun, 18 Dec 2016 14:31:27 +0000 Subject: [PATCH] pcsx2: Fix "ISO Selector" menu item removal memleak Delete() deletes the menu item but keeps the sub menu. Remove() doesn't delete the menu item. Also use AppendSubMenu - using Append on a submenu is deprecated. --- pcsx2/gui/App.h | 1 - pcsx2/gui/MainFrame.cpp | 8 ++------ pcsx2/gui/MainFrame.h | 1 + pcsx2/gui/MainMenuClicks.cpp | 7 +++++-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/pcsx2/gui/App.h b/pcsx2/gui/App.h index c08add86d4..276a04d2e1 100644 --- a/pcsx2/gui/App.h +++ b/pcsx2/gui/App.h @@ -84,7 +84,6 @@ enum MenuIdentifiers MenuId_Src_Plugin, MenuId_Src_NoDisc, MenuId_Boot_Iso, // Opens submenu with Iso browser, and recent isos. - MenuId_IsoSelector, // Contains a submenu of selectable "favorite" isos MenuId_RecentIsos_reservedStart, MenuId_IsoBrowse = MenuId_RecentIsos_reservedStart + 100, // Open dialog, runs selected iso. MenuId_Boot_CDVD, diff --git a/pcsx2/gui/MainFrame.cpp b/pcsx2/gui/MainFrame.cpp index c91d546881..6da502dbf6 100644 --- a/pcsx2/gui/MainFrame.cpp +++ b/pcsx2/gui/MainFrame.cpp @@ -130,8 +130,7 @@ void MainEmuFrame::OnCloseWindow(wxCloseEvent& evt) sApp.OnMainFrameClosed( GetId() ); - if( m_menubar.FindItem(MenuId_IsoSelector) ) - m_menuCDVD.Remove(MenuId_IsoSelector); + RemoveCdvdMenu(); RemoveEventHandler( &wxGetApp().GetRecentIsoManager() ); wxGetApp().PostIdleAppMethod( &Pcsx2App::PrepForExit ); @@ -458,10 +457,7 @@ MainEmuFrame::MainEmuFrame(wxWindow* parent, const wxString& title) wxMenu& isoRecents( wxGetApp().GetRecentIsoMenu() ); //m_menuCDVD.AppendSeparator(); - // FIXME: VS2015 thinks there's a memory issue here and there probably is one. - // The submenu is owned by a unique_ptr, but any menu that is attached to a - // menubar or another menu will be deleted by its parent. - m_menuCDVD.Append( MenuId_IsoSelector, _("ISO &Selector"), &isoRecents ); + m_menuItem_RecentIsoMenu = m_menuCDVD.AppendSubMenu(&isoRecents, _("ISO &Selector")); m_menuCDVD.Append( GetPluginMenuId_Settings(PluginId_CDVD), _("Plugin &Menu"), m_PluginMenuPacks[PluginId_CDVD] ); m_menuCDVD.AppendSeparator(); diff --git a/pcsx2/gui/MainFrame.h b/pcsx2/gui/MainFrame.h index 2ab6b3b045..956344300c 100644 --- a/pcsx2/gui/MainFrame.h +++ b/pcsx2/gui/MainFrame.h @@ -115,6 +115,7 @@ protected: wxMenu& m_LoadStatesSubmenu; wxMenu& m_SaveStatesSubmenu; + wxMenuItem* m_menuItem_RecentIsoMenu; wxMenuItem& m_MenuItem_Console; #if defined(__unix__) wxMenuItem& m_MenuItem_Console_Stdio; diff --git a/pcsx2/gui/MainMenuClicks.cpp b/pcsx2/gui/MainMenuClicks.cpp index 1c73c5e2f5..bd149f3215 100644 --- a/pcsx2/gui/MainMenuClicks.cpp +++ b/pcsx2/gui/MainMenuClicks.cpp @@ -101,8 +101,11 @@ static void WipeSettings() void MainEmuFrame::RemoveCdvdMenu() { - if( wxMenuItem* item = m_menuCDVD.FindItem(MenuId_IsoSelector) ) - m_menuCDVD.Remove( item ); + // Delete() keeps the sub menu and delete the menu item. + // Remove() does not delete the menu item. + if (m_menuItem_RecentIsoMenu) + m_menuCDVD.Delete(m_menuItem_RecentIsoMenu); + m_menuItem_RecentIsoMenu = nullptr; } void MainEmuFrame::Menu_ResetAllSettings_Click(wxCommandEvent &event)