How to manually destroy wxMenu

Question

I have a wxAuiToolBar button which when clicked shows a menu. The menu also has a sub-menu which carries information on recently opened files.

The following code is called when the user clicks on a button on wxAuiToolBar:

m_MainMenu = new wxMenu();
m_RecentFilesMenu = new wxMenu();
wxMenuItem* Item = nullptr;


Item = m_MainMenu->Append(ID_OPEN_FILE, "Open...");
Item->SetBitmap(wxArtProvider::GetBitmap(wxART_FILE_OPEN));

for (const auto& path : m_RecentFiles->GetList())
{
    if (!std::filesystem::exists(path))
        continue;

    int ID = wxNewId();

    wxString p = path.wstring();
    m_RecentFilesMenu->Append(ID, p);

    wxObject* obj = (wxObject*)(new wxString(p));
    m_RecentFilesMenu->Bind(wxEVT_COMMAND_MENU_SELECTED, &pnlScriptEditor::OnRecentFiles, this, ID, wxID_ANY, obj);
}

m_MainMenu->AppendSubMenu(m_RecentFilesMenu, "Recent Files");

    
m_MainMenu->Bind(wxEVT_COMMAND_MENU_SELECTED, &pnlScriptEditor::OnOpen, this, ID_OPEN_FILE);

PopupMenu(m_MainMenu);

Things work as expected; however, I think m_MainMenu is leaking the memory since wxWidgets does not take ownership for PopupMenu. If I call delete m_MainMenu the program crashes due to access violation. Note that in this case it is not possible to allocate m_MainMenu on stack.

How to manually destroy m_MainMenu or should I destroy it?

---

EDIT #1:

1. If m_MainMenu is declared on the stack then as soon as the popup menu closes the following error is issued in debug mode: "Exception thrown at 0x000001ED1D2852A0 in sciencesuit.exe: 0xC0000005: Access violation executing location "

2. Commenting out m_RecentFilesMenu->Bind part in the for loop then there is no crash.

3. Removing obj which has been created using wxObject* obj = (wxObject*)(new wxString(p)); from m_RecentFilesMenu->Bind prevents the crash.

I assume since wxString is not inherited from wxObject, during cleanup unexpected access violation happens. For example, instead of casting wxString to wxObject, if the following is done, then there is no crash.

class RecentFileData :public wxObject
{
public:
    RecentFileData(const std::filesystem::path& path)
    {
        m_Data = path;
    }

    auto GetPath() const
    {
        return m_Data;
    }

private:
    std::filesystem::path m_Data;
};

then,

auto Data = new RecentFileData(path);
m_RecentFilesMenu->Bind(wxEVT_COMMAND_MENU_SELECTED, &pnlScriptEditor::OnRecentFiles, this, ID, wxID_ANY, Data);

Since RecentFileData inherits from wxObject, wxWidgets can nicely do its internal cleaning up.

---

EDIT #2

Based on suggestion from VZ, the following way of handling greatly simplifies the code:

if (m_RecentFilesMenu)
    m_MainMenu->Destroy(ID_RECENTFILES);

m_RecentFilesMenu = new wxMenu();

for (const auto& path : m_RecentFiles->GetList())
{
    if (!std::filesystem::exists(path))
        continue;

    int ID = wxNewId();

    m_RecentFilesMenu->Append(ID, path.wstring());
    m_RecentFilesMenu->Bind(wxEVT_COMMAND_MENU_SELECTED, [this, path](wxCommandEvent& CmdEvt)
    {
        try
        {
            OpenScriptFile(path);
        }
        catch (const std::exception& e)
        {
            wxMessageBox(e.what());
        }
    }, ID);
}

m_MainMenu->Append(ID_RECENTFILES,"Recent Files", m_RecentFilesMenu);

Note:

1. m_MainMenu is allocated on the heap therefore some menu items of the m_MainMenu are re-used and only the submenu is destroyed and re-created.
2. Since Bind uses lambda expression there is no need for RecentFileData class.
3. If the capture is done using reference [&] the program crashes. Therefore, a copy of the path variable must be allowed to be created by the lambda expression thus [this, path] capture.

Answer 1

This code indeed leaks a wxMenu pointer and you need to add delete m_MainMenu; after the call to PopupMenu() to fix it (or use std::unique_ptr<> instead of manually allocating and freeing memory, of course). The crash in the original version is indeed due to deleting a wxString via a pointer to wxObject which is invalid ("undefined behaviour") and can't be done. Your solution with a wrapper class deriving from wxObject is fine, even if there are fancier and arguably nicer ways to avoid using client data pointer completely (define a lambda and capture the data in it instead).

But if this is really how your code looks like, I see no reason not to allocate m_MainMenu on the stack, and it's not clear why do you think it's impossible.

However creating popup menus on the heap is supported and actually pretty useful if creating it takes some time, as you can create it only once and then show it multiple times later.