[vtk-developers] Patch for vtkPythonUtil.cxx

[Prabhu Ramachandran]

Chris Kruszynski wrote:
> I have observed this problem some time ago, it would actually need a 
> second patch to fix all occurrences (or just another one?), which I have 
> attached.
> 
> The test case is easy:
[...]

Excellent!  I can confirm that this bug is present and your patch fixes 
the problem.  I'll check this in along with the test case and your 
description below.  Thanks.

> This should cause a crash upon termination (trying to delete python 
> object after interpreter shutdown), or an assertion error if the 
> observed event is 'DeleteEvent' or 'AnyEvent' (trying to invoke python 
> method after interpreter shutdown). The test case basically consists of 
> a python class which contains a vtk object which is being observed by 
> that same class. There is no error if the event handler is a member of a 
> different class, even if the instance of that class is contained by the 
> same python object which contains the vtk object. There is also no error 
> if a vtk object is subclassed in python and that subclass has an event 
> handler for itself (self.AddObserver(Event, self.Handler)). Finally, the 
> problem disappears if the container class has a cyclic reference to 
> itself (self.CyclicReference = self).

This explains why I never saw this particular bug.  We didn't have any 
observers on the same class for this would actually create a cyclic 
reference and this reference cycle cannot be garbage collected!  This is 
unlike the case when you have self.CyclicReference = self.

> The problem can actually be pretty widespread since it affects all 
> python classes which both contain and observe a vtk object.

Indeed.

> Before the changes to reference counting in revision 1.84 of 
> vtkPythonUtil.cxx, the problem was masked, because an extra reference to 
> the vtk object would remain somewhere, and the vtk object would never be 
> deleted (and thus RemoveAllObservers would never be called, and 
> vtkPythonCommand would never try to delete the python callable object 
> after interpreter shutdown).

I'm not so sure about this.  The reason is that even with older versions 
of VTK you could do this:

import vtk

def f(o, e):
     print e

a = vtk.vtkObject()
a.AddObserver('DeleteEvent', f)
del a

This prints "DeleteEvent", which means that 'a' was indeed deleted.  So 
it wasn't as if there was a memory leak.

In addition, even when you add a cyclic reference, Py_IsInitialized is 
still 0 and the destructor is still called.  So adding the reference 
cycle did not prevent the vtkPythonHashDelete from being called.  If 
vtkPythonCommand::~vtkPythonCommand is called in both cases, why is it 
crashing in one case and not in another?

If you notice, the crash is always in Py_Finalize() the documentation 
for which has a nice section on this:

"""Bugs and caveats: The destruction of modules and objects in modules 
is done in random order; this may cause destructors (__del__() methods) 
to fail when they depend on other objects (even functions) or modules. 
Dynamically loaded extension modules loaded by Python are not unloaded. 
Small amounts of memory allocated by the Python interpreter may not be 
freed (if you find a leak, please report it). Memory tied up in circular 
references between objects is not freed. Some memory allocated by 
extension modules may not be freed. Some extensions may not work 
properly if their initialization routine is called more than once; this 
can happen if an application calls Py_Initialize() and Py_Finalize() 
more than once. """

In our case the VTK objects are created from extension modules which 
means their memory isn't freed at the time Py_Finalize is called.
So the problem seems to be that if there is a collectable reference 
cycle then the memory of the object is not freed, which explains why the 
self.CyclicReference case works.  Some memory is freed, so it looks like 
if the reference cycle is not detected, the memory is freed leading to 
our bug when we deallocate the memory for this->obj twice in the decref 
call.  I haven't dug deeper into the Python internals to verify this 
though so it may be something else entirely.  My brief reading of this 
suggests that there are dark areas of Py_Finalize that I'd rather not 
get into ATM.

Anyway, I'll check in your fix shortly.

cheers,
prabhu