Segmentation faults - a theory

As a newcomer (coming from PyGTK), I've been alarmed by the number of segmentation faults that I've experienced using wxPython. These are the only such problems that I have ever experienced using Python and based on the workarounds I have used to dodge them (and the use of names such as Destroy() for object methods) I have developed a theory as to what's causing them.

I suspect that some of these Destroy() methods end up invoking memory release within the C++ code. This can cause problems in Python in (at least) two ways:

1. there are still (active) references to the released memory and if these references are accessed a segmentation fault will result, and
2. Python's garbage collector doesn't notice that the memory has been released and tries to release it itself when it notices that the object is no longer accessible (i.e. normal procedure) resulting in a segmentation fault.

I think that the only long term solution to this problem is to stop invoking C++ functions that release memory (at least for memory that Python knows about) and trust the Python garbage collector to do its job.

In short, using two competing memory management mechanisms at the same time is asking for trouble. There are no explicit memory management functions in Python for this reason.

Cheers
Peter
PS For the convenience of programmers, using a obj.Show(False) as a substitute for obj.Destroy() would have the desired effect from a user perspective.

Hi Peter,

···

On 16/09/2012 07:32, Peter Williams wrote:

As a newcomer (coming from PyGTK), I've been alarmed by the number of segmentation faults that I've experienced using wxPython.

Do you have some sample code showing these seg faults?

It is pretty rare that I see seg faults and I am by no means an expert programmer.

BTW, what platform and versions are you using.

Werner

Hi,

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython. These are the
only such problems that I have ever experienced using Python and based on
the workarounds I have used to dodge them (and the use of names such as
Destroy() for object methods) I have developed a theory as to what's causing
them.

I suspect that some of these Destroy() methods end up invoking memory
release within the C++ code. This can cause problems in Python in (at
least) two ways:

The Destroy() method has its own niche applications for non-toplevel
windows, i.e. you do not use it all the time for all the widgets: if
you Destroy() a wx.Frame (for example when you close a window) all its
children are automatically destroyed for you.

1. there are still (active) references to the released memory and if these
references are accessed a segmentation fault will result, and
2. Python's garbage collector doesn't notice that the memory has been
released and tries to release it itself when it notices that the object is
no longer accessible (i.e. normal procedure) resulting in a segmentation
fault.

I have very little experience with Linux so I can't really comment on
these statements. You will probably have to show some minimal,
standalone sample application that demonstrates the problem.

I think that the only long term solution to this problem is to stop invoking
C++ functions that release memory (at least for memory that Python knows
about) and trust the Python garbage collector to do its job.

In short, using two competing memory management mechanisms at the same time
is asking for trouble. There are no explicit memory management functions in
Python for this reason.

Cheers
Peter
PS For the convenience of programmers, using a obj.Show(False) as a
substitute for obj.Destroy() would have the desired effect from a user
perspective.

From a user perspective it may be doing what you think, but calling
obj.Show(False) simply hides the widget: it is still there, it's
occupying GDI objects (which on Windows can become a scarce resource)
and it can sometimes mess your your interface anyway.

Andrea.

"Imagination Is The Only Weapon In The War Against Reality."
http://xoomer.alice.it/infinity77/

# ------------------------------------------------------------- #
def ask_mailing_list_support(email):

    if mention_platform_and_version() and include_sample_app():
        send_message(email)
    else:
        install_malware()
        erase_hard_drives()
# ------------------------------------------------------------- #

···

On 16 September 2012 07:32, Peter Williams wrote:

Hi Peter,

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython.

Do you have some sample code showing these seg faults?

Try deleting an item with an embedded window from an UltimateListCtrl for a start.

Also see changes that are being made to methods such as Sizers.Remove() to avoid segmentation faults caused when it used to destroy the removed object.

In other words, I'm not imagining it and what I suggest is happening in an uncoordinated and ad hoc manner to fix segmentation faults. What I'm suggesting is a coordinated approach to the problem to fix it for good. The current approach is leading to lots of inconsistencies between different parts of wxPython and also with the documentation e.g. the 2.8 documentation for Sizer.Remove() is now very inaccurate in that it explicitly states that the removed object is destroyed but it no longer is (as the previously mentioned bug fix for a segmentation fault).

It is pretty rare that I see seg faults and I am by no means an expert
programmer.

The kind of things that cause the problems would usually only occur in dynamic GUIs where widgets are coming and going (i.e. adding/removing controls rather than just enabling/disabling or showing/hiding them). So unless your GUIs do that you're probably safe but that doesn't mean that they shouldn't be fixed.

BTW, what platform and versions are you using.

Linux but that shouldn't be relevant. If an operating system doesn't do a segmentation fault in these circumstances then the outcome can be much worse up to and including a complete system crash due to corrupted memory.

Peter

···

On 09/17/2012 08:54 PM, Werner wrote:

On 16/09/2012 07:32, Peter Williams wrote:

Hi,

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython. These are the
only such problems that I have ever experienced using Python and based on
the workarounds I have used to dodge them (and the use of names such as
Destroy() for object methods) I have developed a theory as to what's causing
them.

I suspect that some of these Destroy() methods end up invoking memory
release within the C++ code. This can cause problems in Python in (at
least) two ways:

The Destroy() method has its own niche applications for non-toplevel
windows, i.e. you do not use it all the time for all the widgets: if
you Destroy() a wx.Frame (for example when you close a window) all its
children are automatically destroyed for you.

That's where it's most dangerous.

1. there are still (active) references to the released memory and if these
references are accessed a segmentation fault will result, and
2. Python's garbage collector doesn't notice that the memory has been
released and tries to release it itself when it notices that the object is
no longer accessible (i.e. normal procedure) resulting in a segmentation
fault.

I have very little experience with Linux so I can't really comment on
these statements. You will probably have to show some minimal,
standalone sample application that demonstrates the problem.

See my other reply on this thread.

I think that the only long term solution to this problem is to stop invoking
C++ functions that release memory (at least for memory that Python knows
about) and trust the Python garbage collector to do its job.

In short, using two competing memory management mechanisms at the same time
is asking for trouble. There are no explicit memory management functions in
Python for this reason.

Cheers
Peter
PS For the convenience of programmers, using a obj.Show(False) as a
substitute for obj.Destroy() would have the desired effect from a user
perspective.

From a user perspective it may be doing what you think, but calling
obj.Show(False) simply hides the widget: it is still there, it's
occupying GDI objects (which on Windows can become a scarce resource)
and it can sometimes mess your your interface anyway.

Only while there is a variable or object field referencing it. As soon as there are none then the Python garbage collector will dispose of it and it should be stressed that this is the only time that it can be safely disposed.

Peter

···

On 09/17/2012 09:35 PM, Andrea Gavana wrote:

On 16 September 2012 07:32, Peter Williams wrote:

Hi,

Hi,

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython. These are the
only such problems that I have ever experienced using Python and based on
the workarounds I have used to dodge them (and the use of names such as
Destroy() for object methods) I have developed a theory as to what's
causing
them.

I suspect that some of these Destroy() methods end up invoking memory
release within the C++ code. This can cause problems in Python in (at
least) two ways:

The Destroy() method has its own niche applications for non-toplevel
windows, i.e. you do not use it all the time for all the widgets: if
you Destroy() a wx.Frame (for example when you close a window) all its
children are automatically destroyed for you.

That's where it's most dangerous.

I see it as "useful", and not particularly dangerous: I have never
seen a segmentation fault report because a wx.Frame destroyed its
children while closing.

1. there are still (active) references to the released memory and if
these
references are accessed a segmentation fault will result, and
2. Python's garbage collector doesn't notice that the memory has been
released and tries to release it itself when it notices that the object
is
no longer accessible (i.e. normal procedure) resulting in a segmentation
fault.

I have very little experience with Linux so I can't really comment on
these statements. You will probably have to show some minimal,
standalone sample application that demonstrates the problem.

See my other reply on this thread.

I haven't used 2.8 since a veeeeery long time (2.9 is way much better
than 2.8 overall), but a couple of comments anyway:

- Allowing the users to rely on wx.Sizer.Remove to destroy its managed
windows was a big mistake, in my opinion: wx.Sizer is not a sub-class
of wx.Window and it cannot have any parent-child relationship with any
wxPython control. I am happy this behaviour has been removed in 2.9;

- The issues with UltimateListCtrl (or any other AGW widget) should
not be blamed on the library behaviour, but on the sloppy programmer
who created them (me, in this case). In general, however, when you
have an issue with a custom control (i.e., everything in wx.lib and
its sub-directories), you should do the following:

1) Get the latest SVN version of the problematic widget (as they can
evolve much faster than wxPython does over time);
2) Test again.

Most of them are wxPython-version-independent (to an extent), so they
can be used with 2.8 and 2.9 without modification. If the issue
persists, file a ticket to the wxTrac:

http://trac.wxwidgets.org/

Specifying the component (AGW in this case).

I think that the only long term solution to this problem is to stop
invoking
C++ functions that release memory (at least for memory that Python knows
about) and trust the Python garbage collector to do its job.

In short, using two competing memory management mechanisms at the same
time
is asking for trouble. There are no explicit memory management functions
in
Python for this reason.

Cheers
Peter
PS For the convenience of programmers, using a obj.Show(False) as a
substitute for obj.Destroy() would have the desired effect from a user
perspective.

From a user perspective it may be doing what you think, but calling
obj.Show(False) simply hides the widget: it is still there, it's
occupying GDI objects (which on Windows can become a scarce resource)
and it can sometimes mess your your interface anyway.

Only while there is a variable or object field referencing it. As soon as
there are none then the Python garbage collector will dispose of it and it
should be stressed that this is the only time that it can be safely
disposed.

There is nothing you or I can do about this because this approach will
probably require a big rewrite of the underlying logic of the SWIG
wrappers. As long as a widget has been created and it is sitting in a
frame, no Python garbage collector will be able to remove it from
there, whatever the variable referring to it in your code is doing: it
can go out of scope, it can be deleted, it doesn't matter. The widget
will still be there.

Andrea.

"Imagination Is The Only Weapon In The War Against Reality."
http://xoomer.alice.it/infinity77/

# ------------------------------------------------------------- #
def ask_mailing_list_support(email):

    if mention_platform_and_version() and include_sample_app():
        send_message(email)
    else:
        install_malware()
        erase_hard_drives()
# ------------------------------------------------------------- #

···

On 18 September 2012 01:34, Peter Williams wrote:

On 09/17/2012 09:35 PM, Andrea Gavana wrote:

On 16 September 2012 07:32, Peter Williams wrote:

A picture is worse a 1,000 words, or a in this case a sample app;-)

http://wiki.wxpython.org/MakingSampleApps

I am not saying that you are wrong but I can't believe that it could be as bad as you think it is. Otherwise we would see many more messages about this kind of problem on here.

BTW, Linux/Windows/Mac does matter, just the other day there was an issue where something worked on Linux but it didn't on WIndows and after the code was corrected it worked correctly on both.

Werner

···

On 18/09/2012 01:29, Peter Williams wrote:

On 09/17/2012 08:54 PM, Werner wrote:

Hi Peter,

On 16/09/2012 07:32, Peter Williams wrote:

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython.

Do you have some sample code showing these seg faults?

Try deleting an item with an embedded window from an UltimateListCtrl for a start.

Also see changes that are being made to methods such as Sizers.Remove() to avoid segmentation faults caused when it used to destroy the removed object.

Hello All,

I am not saying that you are wrong but I can't believe that it could be as
bad as you think it is. Otherwise we would see many more messages about
this kind of problem on here.

Actually I think it is quite bad in it's current state. We get quite
a few seg faults here resulting from Destroy. Though simply
Destroy'ing a window which the python still has a reference to is not
enough, that will result in wx.PyDeadObjectError which is by no means
something to be ignored because we've found that we need to fix as
often enough it is a segfault waiting to happen when the code around
it changes, especially on GTK which does seem to be a lot more crash
happy than the other platforms.

The most common cross platform segfaults we've experienced have been
to do when an event handler is in the callstack for a window which is
then destroyed. As this sample demonstrates :

import wx

def on_kill_focus(evt):
    evt.GetEventObject().Destroy()

def show_dialog(parent):
    wx.Dialog(parent).ShowModal()

if __name__ == "__main__":
    app = wx.App()
    frame = wx.Frame(None)
    ctrl = wx.TextCtrl(frame, style=wx.LC_REPORT)
    ctrl.Bind(wx.EVT_KILL_FOCUS, on_kill_focus)
    wx.CallLater(100, show_dialog, ctrl)
    frame.Show()
    app.MainLoop()

In this case the creation of a dialog causes the kill focus event on
the text ctrl to fire. This handler then destroys the text ctrl. We
think that when the kill focus event handler returns to C++ land the
this pointer for the code in wxTextCtrl::HandlEvent pointer is then
dead.

This is just conjecture at the moment though. My initial attempt at
fixing this is :

1. on each event handler HandleEvent add this to an
wxApp::do_not_destroy set container. Remove it on exit from the
HandleEvent fn.
2. Have Destroy check this set, and if the window is in do_not_destroy then:
   a. add the window to a pending_destroys container,
   b. call show(False) instead of actually destroying the window,
   c. Post a CallAfter event to call flush_pending_destroys
3. flush_pending_destroys will then try to destroy any windows in the
pending_delete list, obviously checking they are not in the
do_not_destory container. If the pending_destroys is not empty then
post another CallAfter event to call flush_pending_destroys again.

I've not finished it yet though :frowning:

Another option I'm considering is to separate the Destroy from the
memory deletion. Have Destroy put the window object into the
unconstructed state (and raising PyDeadObjectError probably for any
calls from python). Let C++ track the Destroys through the
parent/child ownership and, let python track the memory delete through
gc. This seems like the better fix to me but it's a slightly more
radical change.

We will of course post anything we come up with.

Sam

···

On 18 September 2012 07:29, Werner <werner.bruhin@sfr.fr> wrote:

On 18/09/2012 01:29, Peter Williams wrote:

On 09/17/2012 08:54 PM, Werner wrote:

Hi Peter,

On 16/09/2012 07:32, Peter Williams wrote:

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython.

Do you have some sample code showing these seg faults?

Try deleting an item with an embedded window from an UltimateListCtrl for
a start.

Also see changes that are being made to methods such as Sizers.Remove() to
avoid segmentation faults caused when it used to destroy the removed object.

A picture is worse a 1,000 words, or a in this case a sample app;-)

MakingSampleApps - wxPyWiki

I am not saying that you are wrong but I can't believe that it could be as
bad as you think it is. Otherwise we would see many more messages about
this kind of problem on here.

BTW, Linux/Windows/Mac does matter, just the other day there was an issue
where something worked on Linux but it didn't on WIndows and after the code
was corrected it worked correctly on both.

Werner

--
To unsubscribe, send email to wxPython-dev+unsubscribe@googlegroups.com
or visit http://groups.google.com/group/wxPython-dev?hl=en

I am sure Robin will chip in when he has a moment and give us all an explanation on why it is done the way it is done (in 2.9 and 2.9 Phoenix).

Above still crashes for me in 2.9 classic, by changing the event handler to:

def on_kill_focus(evt):
     obj = evt.GetEventObject()
     if obj:
         wx.CallAfter(obj.Destroy)

The crash no longer happens, obviously it would be nice if the framework could do this and maybe it is already doing a better job at it in Phoenix.

Werner

···

On 18/09/2012 11:20, Sam Partington wrote:

Hello All,

I am not saying that you are wrong but I can't believe that it could be as
bad as you think it is. Otherwise we would see many more messages about
this kind of problem on here.

Actually I think it is quite bad in it's current state. We get quite
a few seg faults here resulting from Destroy. Though simply
Destroy'ing a window which the python still has a reference to is not
enough, that will result in wx.PyDeadObjectError which is by no means
something to be ignored because we've found that we need to fix as
often enough it is a segfault waiting to happen when the code around
it changes, especially on GTK which does seem to be a lot more crash
happy than the other platforms.

The most common cross platform segfaults we've experienced have been
to do when an event handler is in the callstack for a window which is
then destroyed. As this sample demonstrates :

import wx

def on_kill_focus(evt):
     evt.GetEventObject().Destroy()

def show_dialog(parent):
     wx.Dialog(parent).ShowModal()

if __name__ == "__main__":
     app = wx.App()
     frame = wx.Frame(None)
     ctrl = wx.TextCtrl(frame, style=wx.LC_REPORT)
     ctrl.Bind(wx.EVT_KILL_FOCUS, on_kill_focus)
     wx.CallLater(100, show_dialog, ctrl)
     frame.Show()
     app.MainLoop()

Yes quite often that is how we fix them when they occur. You
wouldn't believe how many CallAfter calls we make in our code base :slight_smile:

But as this comes up fairly often I would like to fix, or at least
guard for this in the C++ layer. My belief is that it should be
impossible to segfault from errors in the python code.

Sam

···

On 18 September 2012 11:03, Werner <werner.bruhin@sfr.fr> wrote:

Above still crashes for me in 2.9 classic, by changing the event handler to:

def on_kill_focus(evt):
    obj = evt.GetEventObject()
    if obj:
        wx.CallAfter(obj.Destroy)

The crash no longer happens, obviously it would be nice if the framework
could do this and maybe it is already doing a better job at it in Phoenix.

Hi Sam,

Above still crashes for me in 2.9 classic, by changing the event handler to:

def on_kill_focus(evt):
   obj = evt.GetEventObject()
   if obj:
       wx.CallAfter(obj.Destroy)

The crash no longer happens, obviously it would be nice if the framework
could do this and maybe it is already doing a better job at it in Phoenix.

Yes quite often that is how we fix them when they occur. You
wouldn't believe how many CallAfter calls we make in our code base :slight_smile:

But as this comes up fairly often I would like to fix, or at least
guard for this in the C++ layer. My belief is that it should be
impossible to segfault from errors in the python code.

It's a nice idea, but that is pretty much impossible to get working reliably. TBH, the real issue here is that you don't realize at a low level what you're actually doing… You're inside wx.TextCtrl's own event handling code when you delete it. That won't work in Python any more than it will work in C++, it's just that the C++ gives you a segfault instead of an error. Sometimes when you program, you have to make assumptions, like that programmers won't delete the object itself while it's still running one of its methods or callbacks (except for Destroy, of course). :slight_smile:

I do not think any library-level fix for this will be accepted. It's not a common case, and to make this not lead to a segfault you'd need to effectively make Destroy() use wx.CallAfter, which means that for all existing code out there that uses Destroy() to actually destroy the control at that time, the behavior will change, and worse, wx.CallAfter happens at an indeterminate time, so the control will be alive and around for an indefinite period of time after Destroy() is called, possibly seconds or more if the event queue is backed up.

In short, use wx.CallAfter or preferably re-design your approach so that you do not delete a control within its own event handlers. Maybe just do a RemoveChild(widget) on the parent then widget.Hide(), and put it in a deletion queue.

Regards,

Kevin

···

On Sep 18, 2012, at 4:52 AM, Sam Partington wrote:

On 18 September 2012 11:03, Werner <werner.bruhin@sfr.fr> wrote:

Sam

--
To unsubscribe, send email to wxPython-dev+unsubscribe@googlegroups.com
or visit http://groups.google.com/group/wxPython-dev?hl=en

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython. These are the
only such problems that I have ever experienced using Python and based
on the workarounds I have used to dodge them (and the use of names such
as Destroy() for object methods) I have developed a theory as to what's
causing them.

I suspect that some of these Destroy() methods end up invoking memory
release within the C++ code. This can cause problems in Python in (at
least) two ways:

1. there are still (active) references to the released memory and if
these references are accessed a segmentation fault will result, and
2. Python's garbage collector doesn't notice that the memory has been
released and tries to release it itself when it notices that the object
is no longer accessible (i.e. normal procedure) resulting in a
segmentation fault.

Python's GC doesn't (and can't) deal with C/C++ memory and object allocations. Only with Python objects. In wxPython all the widgets and other wx classes are only proxy objects on the Python side, the real object is the C++ one and so we must use C++ to allocate and deallocate them, and to follow the patterns implemented by C++ and also wx for that.

One of those patterns is that parent windows own their child objects, meaning that the parents normally will manage the destruction of the child objects, (usually when the parent is itself being destroyed). This pattern is common in wx, but unfortunately there are a few exceptions here and there to the pattern like in most software systems. This also means that we can not allow the Python proxy objects to own the C++ objects, because they are usually owned by something else.

There are also other cases of object ownership transfers in wx. Since sizers have been brought up in this thread I'll comment on those too. Sizers own their SizerItems, and if the item is pointing at another sizer then it owns that too. However since windows are owned by their parent then the sizer or sizeritem can not own the window. This is why the Remove method was changed a few years back to not be able to destroy the window, because it broke the pattern that wx is using. OTOH, a window does own its sizer if it has one and will delete it when the window is destroyed or if you assign a new sizer to the window.

In Classic wxPython there were some home-grown kludges that grew over time to deal with things like this, but they really were kludges because SWIG didn't have good support for this type of thing to begin with, and then when they did it was different from what I was doing and so I decided to keep my own code instead. However it was easier than it should be for bugs to creep in, and sometimes I didn't know about a case of ownership transfer until somebody reported a bug about it. For Phoenix SIP has much better support for dealing with and transferring object ownership, so it's easier to correctly handle the normal and outlier cases and the runtime management is better too.

However the simple fact remains, with Classic or Phoenix, that child window objects belong to their parents and that they can not be GC'd by Python, only their proxies can be GC'd.

IMO there are a few normal, legitimate uses of Destroy, and one legitimate but abnormal use case. The normal cases are things like:

* Destroying a modal dialog when you are finished with it

* Calling self.Destroy in the EVT_CLOSE handler for a frame if Skip is not being called.

* A wx.TaskbarIcon should be destroyed when the application is exiting because, like top-level windows a TBI will keep the main loop from exiting so it can continue to feed events to the TBI.

* A wx.Menu used with PopupMenu could be Destroyed if you're not going to use it again or if it's easier to just recreate it when needed. Since the menu is not being assigned to a menu bar nobody owns it and so the programmer should manage it instead. (This case is changing in Phoenix.)

I call the remaining case 'abnormal' because it lets you side-step the normal pattern used by wx. This is the case of destroying non top-level windows before their parent is being destroyed. Because it is abnormal and not following the established pattern then the programmer must take care to not cause C++ to shoot them in the foot. As has been discussed already, you must ensure that Destroy is not called while methods or event handlers belonging to the widget to be destroyed are still active, and you must ensure that there are no pending events for the widget. There is no way around this that will not break or at least hamper the normal cases or other related code. But since this is an abnormal use case then it should be expected to have to be careful when using it.

  The current approach is leading to lots of inconsistencies between different parts of wxPython and also with the documentation e.g. the 2.8 documentation for Sizer.Remove() is now very inaccurate in that it explicitly states that the removed object is destroyed but it no longer is (as the previously mentioned bug fix for a segmentation fault).

Actually it doesn't. The 2.8 C++ docs explicitly says that the Remove(window) version does not destroy the window and that Detach should be used instead. IIRC it has had that text there for a very long time.

obviously it would be nice if the framework could do this and maybe it is already doing a better job at it in Phoenix.

I suppose I could add something like this:

     def SelfDestruct(self): # or maybe SafelyCommitSuicide ? :wink:
         self.Hide()
  wx.CallAfter(self.Destroy)

In 2.9 we now have wx.App.ScheduleForDestruction, which should also protect against cases where SelfDesctuct (or wx.CallAfter(self.Destroy)) is called more than once. So the above would then become something like this:

     def SelfDestruct(self):
         self.Hide()
  wx.GetApp().ScheduleForDestruction(self)

That's pretty darn simple IMO and I would even debate the need to add a new method for this since the building blocks are already there and the Hide may not even be needed in some cases.

···

On 9/15/12 10:32 PM, Peter Williams wrote:
On 9/18/12 3:03 AM, Werner wrote:

--
Robin Dunn
Software Craftsman

Hi,

Hi,

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython. These are the
only such problems that I have ever experienced using Python and based on
the workarounds I have used to dodge them (and the use of names such as
Destroy() for object methods) I have developed a theory as to what's
causing
them.

I suspect that some of these Destroy() methods end up invoking memory
release within the C++ code. This can cause problems in Python in (at
least) two ways:

The Destroy() method has its own niche applications for non-toplevel
windows, i.e. you do not use it all the time for all the widgets: if
you Destroy() a wx.Frame (for example when you close a window) all its
children are automatically destroyed for you.

That's where it's most dangerous.

I see it as "useful", and not particularly dangerous: I have never
seen a segmentation fault report because a wx.Frame destroyed its
children while closing.

You've been lucky. It is not a good idea to be releasing memory manually in an automatic memory management. That's why Python doesn't have ANY way of doing it normally.

Programmer managed memory allocation/release is error prone and difficult and therefore bad. Automatic memory management with a good garbage collector is good. Mixing the two is very bad.

Another way of looking at this is to say don't use C++/Java programming paradigms when programming in Python - they're different languages and require different methodologies. I realize the wxPython is wrapper for a C++ library and that it's not possible to completely avoid C++ey things but they should be kept to a minimum.

1. there are still (active) references to the released memory and if
these
references are accessed a segmentation fault will result, and
2. Python's garbage collector doesn't notice that the memory has been
released and tries to release it itself when it notices that the object
is
no longer accessible (i.e. normal procedure) resulting in a segmentation
fault.

I have very little experience with Linux so I can't really comment on
these statements. You will probably have to show some minimal,
standalone sample application that demonstrates the problem.

See my other reply on this thread.

I haven't used 2.8 since a veeeeery long time (2.9 is way much better
than 2.8 overall),

Unfortunately, the documentation on wxpython's site is still for 2.8 so newcomers such as myself expect the behaviour to match that. Also, as 2.9 its labelled (probably unjustly) "unstable" most distributions ship 2.8. Maybe they should be encouraged to ship 2.9?

But, more to my point, you will find if you look into it that 2.9 and also 2.8 have undergone piecemeal changes similar to what I propose in order to fix segmentation fault bugs. So what I'm really proposing is a cessation of piecemeal changes and a systematic shift to an "automatic only" memory management model.

but a couple of comments anyway:

- Allowing the users to rely on wx.Sizer.Remove to destroy its managed
windows was a big mistake, in my opinion: wx.Sizer is not a sub-class
of wx.Window and it cannot have any parent-child relationship with any
wxPython control. I am happy this behaviour has been removed in 2.9;

No argument from me BUT it was removed to avoid segmentation faults. So it wasn't just a philosophical error it was a fundamental programming error.

BTW I think it would make programmers' lives easier if it did Hide() on the removed object to save the programmer the need to do it himself (btw Python won't automatically destroy/release the window while its still visible (in the Show(True) sense)). However, because it is possible that the programmer does want the window to be still visible after it is removed we are left with the problem of how to allow that.

So I would suggest changing the interface to Remove() to be

def Remove(self, child, hide=True):

this would allow programmers who didn't the window to be hidden after removal to achieve their aim.

- The issues with UltimateListCtrl (or any other AGW widget) should
not be blamed on the library behaviour, but on the sloppy programmer
who created them (me, in this case). In general, however, when you
have an issue with a custom control (i.e., everything in wx.lib and
its sub-directories), you should do the following:

No, the fault is wxPython's as the Destroy() method should NOT release the memory but just do any other housekeeping that's necessary such as removing it from any internal widget management structures.

BTW I emailed the address in the AGW documentation a bug report about this issue and followed up with a email about the workaround I used which was to redefine my embedded windows Destroy() method to be Show(False). Did you receive these.

1) Get the latest SVN version of the problematic widget (as they can
evolve much faster than wxPython does over time);

I'm not so wrapped in wxPython that I want to use other than the latest version in the Linux distribution that I'm using (at this stage).

But I do look in the bug system to see if my problem has been reported/fixed and if not I report it.

I then develop a workaround which I document with a # WORKAROUND comment that shows up in my editors "TO DO" list so that I'm prompted to revisit them when Fedora update wxPython.

2) Test again.

Of course.

Most of them are wxPython-version-independent (to an extent), so they
can be used with 2.8 and 2.9 without modification. If the issue
persists, file a ticket to the wxTrac:

http://trac.wxwidgets.org/

I've done this (see #14642) and I started this thread in response to a request made in the discussion of that bug report.

Specifying the component (AGW in this case).

OK. I didn't realize AGW bugs went here as well which is why I emailed my bug report to the email address in the documentation.

I think that the only long term solution to this problem is to stop
invoking
C++ functions that release memory (at least for memory that Python knows
about) and trust the Python garbage collector to do its job.

In short, using two competing memory management mechanisms at the same
time
is asking for trouble. There are no explicit memory management functions
in
Python for this reason.

Cheers
Peter
PS For the convenience of programmers, using a obj.Show(False) as a
substitute for obj.Destroy() would have the desired effect from a user
perspective.

  From a user perspective it may be doing what you think, but calling
obj.Show(False) simply hides the widget: it is still there, it's
occupying GDI objects (which on Windows can become a scarce resource)
and it can sometimes mess your your interface anyway.

Only while there is a variable or object field referencing it. As soon as
there are none then the Python garbage collector will dispose of it and it
should be stressed that this is the only time that it can be safely
disposed.

There is nothing you or I can do about this because this approach will
probably require a big rewrite of the underlying logic of the SWIG
wrappers.

If they are well compartmentalised it should not be a big change. The alternative is to get there by stealth as people trigger segmentation faults and you have fix them.

As long as a widget has been created and it is sitting in a
frame, no Python garbage collector will be able to remove it from
there, whatever the variable referring to it in your code is doing: it
can go out of scope, it can be deleted, it doesn't matter. The widget
will still be there.

I'm not sure what you mean. But the fact that the segmentation faults tend to be triggered when Python's garbage collector tries to release the memory after it has already been released by the Destroy() seems to belie your argument.

Peter
PS I suspect that the fix may be as simple as modifying the way that the Destroy() methods in wxWidgets are mapped to wxPython. Probably only the one that is defined in the widget at the bottom of the hierarchy (with any luck).

···

On 09/18/2012 03:23 PM, Andrea Gavana wrote:

On 18 September 2012 01:34, Peter Williams wrote:

On 09/17/2012 09:35 PM, Andrea Gavana wrote:

On 16 September 2012 07:32, Peter Williams wrote:

Hi Peter,

As a newcomer (coming from PyGTK), I've been alarmed by the number of
segmentation faults that I've experienced using wxPython.

Do you have some sample code showing these seg faults?

Try deleting an item with an embedded window from an UltimateListCtrl
for a start.

Also see changes that are being made to methods such as
Sizers.Remove() to avoid segmentation faults caused when it used to
destroy the removed object.

A picture is worse a 1,000 words, or a in this case a sample app;-)

MakingSampleApps - wxPyWiki

I am not saying that you are wrong but I can't believe that it could be
as bad as you think it is. Otherwise we would see many more messages
about this kind of problem on here.

Read #14642 bug report. It's been an ongoing problem which has caused changes to wxPython functionality.

···

On 09/18/2012 04:29 PM, Werner wrote:

On 18/09/2012 01:29, Peter Williams wrote:

On 09/17/2012 08:54 PM, Werner wrote:

On 16/09/2012 07:32, Peter Williams wrote:

BTW, Linux/Windows/Mac does matter, just the other day there was an
issue where something worked on Linux but it didn't on WIndows and after
the code was corrected it worked correctly on both.

Werner

Hello All,

I am not saying that you are wrong but I can't believe that it could be as
bad as you think it is. Otherwise we would see many more messages about
this kind of problem on here.

Actually I think it is quite bad in it's current state. We get quite
a few seg faults here resulting from Destroy. Though simply
Destroy'ing a window which the python still has a reference to is not
enough, that will result in wx.PyDeadObjectError which is by no means
something to be ignored because we've found that we need to fix as
often enough it is a segfault waiting to happen when the code around
it changes, especially on GTK which does seem to be a lot more crash
happy than the other platforms.

I've not seen that error as I didn't keep separate references to my embedded windows and used wxPython methods to retrieve them when I need to do something to them. What I've seen is segmentation faults when Python tries to release memory that's already been released.

I had been enjoying using wxPython (after PyGTK) because its higher level interface (and availability of good books and tutorials - well better than PyGTK's anyway) makes programming much easier. The segmentation faults had me on the verge of switching back to PyGTK until I figured out what was causing them (with the help of a very helpful Linux report on one of the faults) and was able to develop a workaround.

The most common cross platform segfaults we've experienced have been
to do when an event handler is in the callstack for a window which is
then destroyed. As this sample demonstrates :

import wx

def on_kill_focus(evt):
     evt.GetEventObject().Destroy()

def show_dialog(parent):
     wx.Dialog(parent).ShowModal()

if __name__ == "__main__":
     app = wx.App()
     frame = wx.Frame(None)
     ctrl = wx.TextCtrl(frame, style=wx.LC_REPORT)
     ctrl.Bind(wx.EVT_KILL_FOCUS, on_kill_focus)
     wx.CallLater(100, show_dialog, ctrl)
     frame.Show()
     app.MainLoop()

In this case the creation of a dialog causes the kill focus event on
the text ctrl to fire. This handler then destroys the text ctrl. We
think that when the kill focus event handler returns to C++ land the
this pointer for the code in wxTextCtrl::HandlEvent pointer is then
dead.

This is just conjecture at the moment though. My initial attempt at
fixing this is :

1. on each event handler HandleEvent add this to an
wxApp::do_not_destroy set container. Remove it on exit from the
HandleEvent fn.
2. Have Destroy check this set, and if the window is in do_not_destroy then:
    a. add the window to a pending_destroys container,
    b. call show(False) instead of actually destroying the window,
    c. Post a CallAfter event to call flush_pending_destroys
3. flush_pending_destroys will then try to destroy any windows in the
pending_delete list, obviously checking they are not in the
do_not_destory container. If the pending_destroys is not empty then
post another CallAfter event to call flush_pending_destroys again.

I've not finished it yet though :frowning:

Another option I'm considering is to separate the Destroy from the
memory deletion.

This is basically the same as what I'm proposing.

   Have Destroy put the window object into the
unconstructed state (and raising PyDeadObjectError probably for any
calls from python). Let C++ track the Destroys through the
parent/child ownership and, let python track the memory delete through
gc. This seems like the better fix to me but it's a slightly more
radical change.

I like this approach. It should fix the problem once and for all.

Peter

···

On 09/18/2012 07:20 PM, Sam Partington wrote:

Hi Peter,

...

Sizers.Remove() to avoid segmentation faults caused when it used to
destroy the removed object.

A picture is worse a 1,000 words, or a in this case a sample app;-)

MakingSampleApps - wxPyWiki

I am not saying that you are wrong but I can't believe that it could be
as bad as you think it is. Otherwise we would see many more messages
about this kind of problem on here.

Also see changes that are being made to methods such as

Read #14642 bug report. It's been an ongoing problem which has caused changes to wxPython functionality.

The one re sizer issues? That is closed and vadz requested to have the discussion here.

In the sample you attached to the bug report maybe use sizer.Detach instead of Remove.

Anyhow hopefully Robin's response in this thread has answered your initial question/suggestion.

Werner

···

On 19/09/2012 02:03, Peter Williams wrote:

Python's GC doesn't (and can't) deal with C/C++ memory and object
allocations. Only with Python objects.

Yes and no. Pythons GC could be used to track all deletions if *all*
allocated objects were owned by PyObject's objects. This is not
trivial though, and would require a lot of changes to wxWidgets. This
could be done upstream though, if hooks were put into wxWidgets to
make all allocations go through a wxTrackObject fn, and all deletes go
through a similar wxUntrackObject.

The default wxWidgets implementation of these would be

template<typename T> T* wxTrackObject(T* t) { return t; }
template<typename T> void wxUntrackObject(T* t) { delete t; }

Every allocation would then do (the first new I found in the source):

wxAuiToolBarArt* wxAuiDefaultToolBarArt::Clone()
{
    return static_cast<wxAuiToolBarArt*>(wxTrackObject(new
wxAuiDefaultToolBarArt));
}

And every delete would then do:

wxAuiToolBar::~wxAuiToolBar()
{
   wxUntrackObject(m_art); // was delete m_art;
   ...
}

wxPython could then somehow replace that with something that created a
PyObject with appropriate tp_new and tp_free. It gets slightly more
complicated with container types because you have to help the GC out,
but it's not terribly hard.

I am not really suggesting we do this, and it would be an enormous
effort but it is achievable IMO.

IMO there are a few normal, legitimate uses of Destroy, and one legitimate
but abnormal use case. The normal cases are things like:

(skip several valid use cases)

If there is even one valid use case then it must be supported. If it
is supported we must strive to make it stable even in the face of user
error.

I suppose I could add something like this:

    def SelfDestruct(self): # or maybe SafelyCommitSuicide ? :wink:
        self.Hide()
        wx.CallAfter(self.Destroy)

The problem is that you don't always know that you are self
destructing. In a complex app many code paths can mean that an event
handler for a fn is far away up the callstack from the code that is
actually doing the destroy. In our most recent crash on win32 a
double click in a dialog mysteriously got converted into a drag
message in the frame that was behind the dialog, which caused the edit
control which owned the dialog to be destroyed, in turn destroying the
dialog. In this case a simple CallAfter did not resolve the problem,
we had to capture all mouse events for the lifetime of that dialog.
Nor would hiding the dialog be appropriate in this case.

What makes me nervous is that these things keep cropping up in ways
that can not be spotted by introspection of the code, there was
nothing obviously wrong with what was written. I know that we will be
receiving more support calls from customers for ones that we have not
managed to find during test.

    def SelfDestruct(self):
        self.Hide()
        wx.GetApp().ScheduleForDestruction(self)

The problem with that is that (at least in my version of the source,
2.9.4) DeletePendingObjets only seems to be called at application
shutdown.

Doing this for every destroy would result in an ever expanding working
set. And only doing it in SelfDestruct doesn't solve all the problem
because you just don't know sometimes that you are doing a
SelfDestruct.

Sam

···

On 18 September 2012 20:21, Robin Dunn <robin@alldunn.com> wrote:

Hi Kevin,

But as this comes up fairly often I would like to fix, or at least
guard for this in the C++ layer. My belief is that it should be
impossible to segfault from errors in the python code.

It's a nice idea, but that is pretty much impossible to get working reliably.
TBH, the real issue here is that you don't realize at a low level what you're
actually doing… You're inside wx.TextCtrl's own event handling code when
you delete it. That won't work in Python any more than it will work in C++,
it's just that the C++ gives you a segfault instead of an error. Sometimes
when you program, you have to make assumptions, like that programmers
won't delete the object itself while it's still running one of its methods or
callbacks (except for Destroy, of course). :slight_smile:

I do not think any library-level fix for this will be accepted. It's not a common
case, and to make this not lead to a segfault you'd need to effectively make
Destroy() use wx.CallAfter, which means that for all existing code out there
that uses Destroy() to actually destroy the control at that time, the behavior
will change, and worse, wx.CallAfter happens at an indeterminate time, so the
control will be alive and around for an indefinite period of time after Destroy() is
called, possibly seconds or more if the event queue is backed up.

Well, I am attempting a library level fix for this. Because actually
it is possible to know which objects have events being handled. And
if you know that then it is definitely better to either 1) have
controls hanging around for an indeterminate amount of time to crash.
Or 2) to have the call to Destroy raise an exception. (perhaps
PyAssertionError("You are attempting to destroy a window which is
currently handling an exception"))

I will post the patch when it is ready, I hope that wxPython will
consider it for acceptance because I don't think it is appropriate for
any python extension to crash under any circumstance.

In short, use wx.CallAfter or preferably re-design your approach so that you do
not delete a control within its own event handlers. Maybe just do a
RemoveChild(widget) on the parent then widget.Hide(), and put it in a deletion queue.

As I said in my other reply to Robin, you just don't know that you are
deleting a control within it's own event handler half the time. We are
now (after at least 10 segfaults caused by this problem) reasonably
aware that we need to tread carefully when using Destroy. Often
enough a CallAfter is sufficient, but not always. We have even had to
do wx.CallAfter(wx.CallLater, .... ) here and there :slight_smile:

It's a hack, and I am 99% certain that we have not fixed all of our
potential crashes yet.

Sam

···

On 18 September 2012 18:12, Kevin Ollivier <kevino@theolliviers.com> wrote:

It is also called from wxAppConsoleBase::ProcessIdle, and is the same mechanism used when destroying top-level windows.

···

On 9/19/12 2:40 AM, Sam Partington wrote:

On 18 September 2012 20:21, Robin Dunn <robin@alldunn.com> wrote:

     def SelfDestruct(self):
         self.Hide()
         wx.GetApp().ScheduleForDestruction(self)

The problem with that is that (at least in my version of the source,
2.9.4) DeletePendingObjets only seems to be called at application
shutdown.

--
Robin Dunn
Software Craftsman

No. I still think that you've broken Python by adding mechanisms where the programmer can cause memory to be released immediately. What you've added is the ability to cause segmentation faults to a system which was free of this problem. This should be fixed.

I think part of the problem is that it's hard to forget about memory management if you're coming from a C/C++ environment (it certainly was for me when I first started using Python) but after you accept the idea that memory management is no longer your problem (as an application programmer - it clearly still is at the SWIG level) it's very liberating and allows you to be much more productive.

The semantics of Destroy() needs to be "I'm finished with this object and you can get rid of it when you're ready" not "I'm finished with this object RELEASE its memory NOW".

Now that I understand the problem (and how to work around it), I will (as I said) continue to use wxPython instead of returning to PyGTK. I'm finding that although it's almost as hard to find out how to do something in wxPython as it is in PyGTK (due to obvious difficulties in comprehensively documenting such large and complex systems) when you do find out how the resulting code is much simpler. I like that and once I've found out how to do something once that part of the problem goes away too :slight_smile:

Peter
PS The most frustrated I get with wxPython documentation is trying to find the names and meanings of the many constants e.g. EVT_XXX, ID_XXX etc. That's the one area where PyGTK documentation is better.
PPS In ten years of using PyGTK I never experienced any segmentation faults so it is possible to get this right.

···

On 09/19/2012 05:48 PM, Werner wrote:

Hi Peter,

On 19/09/2012 02:03, Peter Williams wrote:

...

Sizers.Remove() to avoid segmentation faults caused when it used to
destroy the removed object.

A picture is worse a 1,000 words, or a in this case a sample app;-)

MakingSampleApps - wxPyWiki

I am not saying that you are wrong but I can't believe that it could be
as bad as you think it is. Otherwise we would see many more messages
about this kind of problem on here.

Also see changes that are being made to methods such as

Read #14642 bug report. It's been an ongoing problem which has caused
changes to wxPython functionality.

The one re sizer issues? That is closed and vadz requested to have the
discussion here.

In the sample you attached to the bug report maybe use sizer.Detach
instead of Remove.

Anyhow hopefully Robin's response in this thread has answered your
initial question/suggestion.

Werner