Segmentation Faults with wxMenu.RemoveItem - Appears to work as expected in straight wxWidgets

wxPython Users
1

* My apologies if this is a repost. I tried emailing the list from a
different account, and I didn't see this go through.

I’ve been getting unexpected segmentation faults when using the
RemoveItem/Remove methods in wxPython (2.8.10 and 2.9.0) in conjuction
with AppendItem. I’m going to attempt to attach a python script that
seg faults under both Windows and Linux GTK, but in case the
attachments don’t come through, the code is copied and pasted below.
The first script is the wxPython version that seg faults. Simply
click the button and the handler will attempt to remove the item from
the file menu and then will re-add it. Once the item has been removed
and re-added, the menu becomes corrupted and a second click or simply
expanding the menu will result in a seg fault. Now the one weird
thing I noticed is that if I explicity pass in the original reference
of the menu item I added instead of getting a new reference from
wxMenu.GetMenuItems(), the seg fault is avoided. To illustrate this
behavior, uncomment the block of code in the button handler that is
prefaced with the comment “This Works”. The second source file is a
pure wxWidgets implementation that sets up a similar scenario and
works. To execute the handler that removes and re-adds the menu
items, click the item under the edit menu, and the handler will remove
and re-add the items in the file menu.

Per the wxWidgets documentation, wxMenu.Remove() shouldn’t delete the
underlying menu item, so I should be able to re-add it.

Josh

##############Begin
test.py####################################################
import wx
import sys
class MyFrame(wx.Frame):

    def __init__(self, parent, *args, **kwargs):
        wx.Frame.__init__(self, parent, *args, **kwargs)

        panel = wx.Panel(self)
        panel.SetBackgroundColour('pink')
        button = wx.Button(panel, wx.ID_ANY, "HI", pos=(100, 100))
        panel.Bind(wx.EVT_BUTTON, self.OnClick)

        sizer = wx.BoxSizer(wx.VERTICAL)
        sizer.Add(panel, 1, wx.EXPAND)
        self.SetSizer(sizer)

        self.menu = wx.Menu()
        self.menuBar = wx.MenuBar()
        self.menuBar.Append(self.menu, "Test")

        self.item = wx.MenuItem(self.menu, wx.ID_ANY, "I will be
removed/added on button click")
        self.menu.AppendItem(self.item)
        self.SetMenuBar(self.menuBar)

    def OnClick(self, event):
        print >>sys.stderr, "OnClick()", self.item
        for item in self.menu.GetMenuItems():
            print >>sys.stderr, "Removing Item", item, "...",
self.item
            self.menu.RemoveItem(item)

         #This doesn't work either
# for item in self.menu.GetMenuItems():
# print >>sys.stderr, "Removing Item", item.GetId(), "...",
self.item.GetId()
# self.menu.Remove(item.GetId())

        #This Works
# print >>sys.stderr, "Removing Item", self.item, "..."
# self.menu.RemoveItem(self.item)

        print >>sys.stderr, "Readding Item...", self.item
        self.menu.AppendItem(self.item)

if __name__ == "__main__":

    app = wx.App(0)
    frame = MyFrame(None, title="HELLO")
    frame.SetSize((600, 400))
    frame.Show()
    app.MainLoop()

#################################End
test.py############################################

////////////////////////////////////////Begin
wx.cpp//////////////////////////////////////////////////////////

#include "wx/wx.h"

#include <iostream>
using std::cout;
using std::endl;

class MyApp: public wxApp
{
    virtual bool OnInit();
};

class MyFrame: public wxFrame
{
public:

    MyFrame(const wxString& title, const wxPoint& pos, const wxSize&
size);

    void OnQuit(wxCommandEvent& event);
    void OnAbout(wxCommandEvent& event);
    void OnHi(wxCommandEvent& event);

    DECLARE_EVENT_TABLE()
protected:
    wxMenu* menuFile;
  wxMenuItem* aboutItem;
  wxMenuItem* exitItem;
};

enum
{
    ID_Quit = 1,
    ID_About,
};

BEGIN_EVENT_TABLE(MyFrame, wxFrame)
    EVT_MENU(ID_Quit, MyFrame::OnQuit)
    EVT_MENU(ID_About, MyFrame::OnAbout)
END_EVENT_TABLE()

IMPLEMENT_APP(MyApp)

bool MyApp::OnInit()
{
    MyFrame *frame = new MyFrame( _T("Hello World"), wxPoint(50,50),
wxSize(450,340) );
    frame->Show(TRUE);
    SetTopWindow(frame);
    return TRUE;
}

MyFrame::MyFrame(const wxString& title, const wxPoint& pos, const
wxSize& size)
: wxFrame((wxFrame *)NULL, -1, title, pos, size)
{
    menuFile = new wxMenu;
    wxMenu *menuEdit = new wxMenu;

    aboutItem = menuFile->Append( ID_About, _T("&About...") );
    exitItem = menuFile->Append( ID_Quit, _T("E&xit") );

    wxMenuItem* item = menuEdit->Append(wxID_ANY, _T("HI"));
    Bind(wxEVT_COMMAND_MENU_SELECTED, &MyFrame::OnHi, this, item->GetId
());

    wxMenuBar *menuBar = new wxMenuBar;
    menuBar->Append( menuFile, _T("&File") );
    menuBar->Append(menuEdit, _T("Edit"));

    SetMenuBar( menuBar );

    CreateStatusBar();
    SetStatusText( _T("Welcome to wxWindows!") );
}

void MyFrame::OnQuit(wxCommandEvent& WXUNUSED(event))
{
    Close(TRUE);
}

void MyFrame::OnAbout(wxCommandEvent& WXUNUSED(event))
{
    wxMessageBox(_T("This is a wxWindows Hello world sample"),
        _T("About Hello World"), wxOK | wxICON_INFORMATION, this);
}

void MyFrame::OnHi(wxCommandEvent& WXUNUSED(event))
{
  cout << "OnHi" << endl;

  wxMenuItemList list = menuFile->GetMenuItems();
  wxMenuItemList::iterator iter = list.begin();
  for(;iter != list.end(); iter++) {
    cout << "Removing: " << (*iter)->GetId() << endl;
    menuFile->Remove(*iter);
  }

  cout << "Readding" << endl;
  menuFile->Append(aboutItem);
  menuFile->Append(exitItem);
}

///////////////////////////////////////////////////End
wx.cpp////////////////////////////////////////////

2

This has to do with how the Python proxy objects work in SWIG generated modules. The menu item object you get from GetMenuItems is not the same Python object as self.item, although they both point to the same C++ object. In your example you can use id() to see that the two menu item objects are not the same Python object. Unless I'm able to help it out in some way (which I do for widgets but can't do for wx.MenuItem) then every time a C++ method returns a pointer to an object then SWIG will make a *new* python proxy for it. Most of the time this is not that much of an issue, as the proxies are normally meant to be used as a transient facade for the real C++ object hidden behind it.

SWIG also keeps track of whether the C++ object is owned by the Python proxy object or not, so it knows if it should attempt to destroy it when the Python proxy goes out of scope, and the wxPython SWIG wrappers are able to use some hints to tell SWIG when that ownership should change differently than the assumptions that it normally makes.

For wx.MenuItem we make the assumption that when an item is appended/inserted to a menu then the menu is the one that owns the item, not the Python proxy object, and so we tell SWIG to 'disown' it. In your example you can see that by looking at self.item.this.own(), it should be False. When an item is removed from the menu then we change the ownership such that the proxy object owns the C++ object, so if the python object goes out of scope at this point then the C++ object will be correctly cleaned up. Unfortunately the RemoveItem method also returns a pointer to the menu item that it removed so we end up with an additional proxy to the menu item, and it is the one that has the un-disowning done to it so if you don't save that reference it will go out of scope and destroy the C++ object, which causes the crash when wx tries to use it later. So I did a little more trickery in the wrapper for RemoveItem to work around the problem, but you must use the return value of RemoveItem for it to work. In your example just saving that return value to self.item takes care of the problem, although if you don't want to save it in self then just using the return value when reappending the item would work too.

     def OnClick(self, event):
         item = self.menu.GetMenuItems()[0]

         # Show that they are different Python objects
         print id(item), id(self.item)

         # the proxies don't own the C++ objects
         print 'before:', item.this.own(), self.item.this.own()

         self.item = self.menu.RemoveItem(item)
         # now we do own it
         print 'after:', self.item.this.own()

         self.menu.AppendItem(self.item)
         # now we don't own it again, the menu does.
         print 'end:', self.item.this.own()

···

On 11/5/09 3:15 PM, Mears wrote:

* My apologies if this is a repost. I tried emailing the list from a
different account, and I didn't see this go through.

I’ve been getting unexpected segmentation faults when using the
RemoveItem/Remove methods in wxPython (2.8.10 and 2.9.0) in conjuction
with AppendItem. I’m going to attempt to attach a python script that
seg faults under both Windows and Linux GTK, but in case the
attachments don’t come through, the code is copied and pasted below.
The first script is the wxPython version that seg faults. Simply
click the button and the handler will attempt to remove the item from
the file menu and then will re-add it. Once the item has been removed
and re-added, the menu becomes corrupted and a second click or simply
expanding the menu will result in a seg fault. Now the one weird
thing I noticed is that if I explicity pass in the original reference
of the menu item I added instead of getting a new reference from
wxMenu.GetMenuItems(), the seg fault is avoided. To illustrate this
behavior, uncomment the block of code in the button handler that is
prefaced with the comment “This Works”. The second source file is a
pure wxWidgets implementation that sets up a similar scenario and
works. To execute the handler that removes and re-adds the menu
items, click the item under the edit menu, and the handler will remove
and re-add the items in the file menu.

Per the wxWidgets documentation, wxMenu.Remove() shouldn’t delete the
underlying menu item, so I should be able to re-add it.

--
Robin Dunn
Software Craftsman