Table in a dialog segfaults under wx, EPD 7

[jdmarch]

The code below segfaults in Ubuntu 10.04 and Mac OSX 10.6 after several iterations of: click button to open table dialog, edit table, close table dialog. Not tested in Windows. The number of iterations before segfault varies depending on OS, which TableEditor options are set, prior state of the terminal session,.... i.e. presumably depending on the state of memory. Under Ubuntu, a faulthandler stack dump shows that the segfault occurs when wx is disposing of a panel.

Corran suspects unresolved deallocation bugs in TableEditor with wx, due to inaccurate manual reference counting of wx's Attr objects, and advises that a leaky workaround can be to double up one or both of the IncRef (increment reference) calls in method _GridTableBase.GetAttr in module traitsbackendwx/enthought/pyface/ui/wx/grid/grid.py.

Another workaround can be to use TabularEditor.

from enthought.traits.api import HasTraits, Button, Str, List
from enthought.traits.ui.api import View, UItem, TableEditor, ObjectColumn

class MyRow(HasTraits):
    name = Str('abc')

class TableView(HasTraits):
    mytable = List

    traits_view = View(
        UItem('mytable',
              editor = TableEditor(
                  columns = [ObjectColumn(name = 'name',
                                           label = 'Property',
                                           width = 300 ),
                             ],
                  #configurable = False,
                  #deletable = True,
                  row_factory = MyRow,
                  #sortable = False,
                  auto_size = False,
                  #orientation = 'vertical',
                  #reorderable = True,
                  #show_toolbar = True,
                  )
              ),
        height = 300
    )

class AppView(HasTraits):
    button = Button
    mytable = List([MyRow(), MyRow()])
    traits_view = View('button')

    def _button_fired(self):
        test_view = TableView(mytable=self.mytable)
        test_view.edit_traits(kind='livemodal')


if __name__ == '__main__':
    import faulthandler
    faulthandler.enable()
    top_view = AppView()
    top_view.configure_traits()

[mdickinson]

On Wed, Jun 15, 2011 at 4:04 PM, jdmarch
reply@reply.github.com
wrote:

The code below segfaults in Ubuntu 10.04 and Mac OSX 10.6 after several iterations of: click button to open table dialog, edit table, close table dialog.

This looks very similar to an issue that I was encountering recently,
and that's a significant problem for a current project. With the code
above, I get a reliable, reproducible segfault on OS X 10.6.7 by
doing:

(1) Run the script from a Terminal prompt. ("python jmarch.py")
(2) In the UI Window, click on the 'Button' button.
(3) Select the second row of the table.
(4) Click on the red 'X' close button at the top left of the UI window.

Mark

[mdickinson]

Hmm. Doubling up the IncRef calls you mention doesn't work for me, though, so maybe I'm seeing a different issue.

[jdmarch]

To clarify: I just switched my code to TabularEditor, mentioned Corran's IncRef suggestion as a possible diagnostic lead, but didn't try it myself yet.

[mdickinson]

Some progress: the patch below (which is definitely an ugly workaround rather than a committable fix) fixes the BusError on my machine. Jonathan, do you have a second to see if this also fixes your issue?

Basically, it seems as though something's trying to access the grid cell elements after the TableEditor has been disposed of. The hack below works by pretending that the number of rows and the number of columns are both zero if the TableModel .editor attribute has been set to None. This fixes the BusError for me.

The right fix would clearly be to understand why Table elements are still being accessed after the editor has been disposed of, and to stop that happening, but that requires more wx and traitsui knowledge than I currently possess (though I'm working on it). Could Bryce maybe shed some light on what's wrong here?

diff --git a/traitsui/wx/table_model.py b/traitsui/wx/table_model.py
index 4f646b4..a7c08ad 100644
--- a/traitsui/wx/table_model.py
+++ b/traitsui/wx/table_model.py
@@ -347,6 +347,10 @@ class TableModel ( GridModel ):
     def get_column_count ( self ):
         """ Returns the number of columns for this table.
         """
+        if self.editor is None:
+            print "In get_column_count and self.editor is None"
+            return 0
+
         return len( self.__get_columns() )

     def get_column_name ( self, index ):
@@ -412,6 +416,10 @@ class TableModel ( GridModel ):
     def get_row_count ( self ):
         """ Return the number of rows for this table.
         """
+        if self.editor is None:
+            print "In get_row_count and self.editor is None"
+            return 0
+
         return len( self.__filtered_items() )

     def get_row_name ( self, index ):

[jdmarch]

Yes, your workaround works around my issue in Ubuntu. No time to test OSX right now. Nice hack.

[jdmarch]

Bryce emailed: """
I'd suggest using a tool Dave Morril wrote, the HeapBrowser. With it, you can create a baseline right after the GUI is displayed, then select a row and look at the delta to see what is holding on to references.
https://github.com/enthought/etsdevtools/blob/master/etsdevtools/developer/tools/heap_browser.py
To use the HeapBrowser, I'd instantiated it and call edit_traits() from the code that launches your GUI.
"""

[WarrenWeckesser]

With the faulthandler code commented out, I get a "Bus error" in OS X 10.5.8 and ETS trunk with following sequence:

1. Click on "Button"
2. Click on the second row of the table.
3. Close the table dialog by clicking on the red x.
4. Click on "Button"

With Mark's patch, I do not get the bus error.

[jdmarch]

Mark, absent a real fix, I think your patch belongs in master - prominently marked as temporary, of course ;)

[mdickinson]

I've been reviewing this. Tried the heap browser but didn't get much usable from that.

One breakthrough: the segfault I was seeing was almost, but not quite, 100% reproducible. I've think I've just figured out the 'almost' part: for me, the segfault is reliably caused by: (1) editing one of the rows of the table, then (2) hovering over a different row before closing the editor. This solves the puzzle of why I was seeing this only when clicking on the second row of the table---my mouse pointer naturally passed over the first row on the way to the close button!

[mdickinson]

Okay, I'm out of time on this for now. Some observations for whoever next has a chance to pick it up again:

1. This is likely a PyFace bug rather than a TraitsUI bug.
2. The root cause of the segmentation fault is that wx doesn't like having multiple PyGridCellEditor objects around for any one cell of the table being edited, so the editors need to be cached. This file below demonstrates this in pure wx code: https://gist.github.com/1300596
   PyFace works around this by caching editors (see _editor_cache in pyface/ui/wx/grid/grid.py). But it fails to deal with a second wx oddity, namely that:
3. In the circumstances that cause the bug (editing one cell, then moving over another before closing the window), the wx grid machinery emits an extra call to GetAttr when the window is closed. Again this is a pure wx thing, and can be seen in the code snippet above. But:
4. With PyFace as it stands, this extra GetAttr call occurs after disposing of the editor and related objects. This disposal kills the editor cache, so the final GetAttr call ends up generating a new editor for the same row and cell as an editor was previously generated for. Then as a result of 1., we get a segfault.

The key part that I still don't understand is why the final GetAttr call occurs after the _GridTableBase.dispose method gets called, or how to fix this. I suspect that either (1) a rearrangement of the cleanup code, or (2) a suitable use of invoke_later might be able to deal with this.

[mdickinson]

Having said that I was out of time, I couldn't resist acting on the hunch that a well-placed 'do_later' (which is what I meant when I mistakenly wrote 'invoke_later' above) might solve this problem. And indeed, the following patch seems to fix things.
I think this one can be regarded as a 'real' fix.

diff --git a/pyface/ui/wx/grid/grid.py b/pyface/ui/wx/grid/grid.py
index b6101d3..b3978f7 100644
--- a/pyface/ui/wx/grid/grid.py
+++ b/pyface/ui/wx/grid/grid.py
@@ -1514,16 +1514,20 @@ class _GridTableBase(PyGridTableBase):
         self._renderer_cache = {}

     def dispose(self):
-
         # Make sure dispose gets called on all traits editors:
-        for editor in self._editor_cache.values():
-            editor.dispose()
-        self._editor_cache = {}

+        # The wx grid machinery can end up calling GetAttr one more time after
+        # this call, so we delay emptying of the editor cache.
+        do_later(self._empty_editor_cache)
         for renderer in self._renderer_cache.values():
             renderer.dispose()
         self._renderer_cache = {}

+    def _empty_editor_cache(self):
+        for editor in self._editor_cache.values():
+            editor.dispose()
+        self._editor_cache = {}
+
     ###########################################################################
     # 'wxPyGridTableBase' interface.
     ###########################################################################

[mdickinson]

Fixed in the PyFace repository:

enthought/pyface@ba4bea0

[mdickinson]

Hmm. That 'fix' proved a little too hasty. It worked for a standalone TableEditor ui, but not for a TableEditor window popped up from another UI component---that gave a different segfault, apparently originating from the DecRef call in trait_grid_cell_adapter.py.

I'd reopen this, except that this does appear to be a pyface issue at heart rather than a Traits UI issue. So I'll open an issue on the pyface tracker.

[mdickinson]

Opened Pyface issue:

enthought/pyface#10