<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
  <title></title>
</head>
<body bgcolor="#ffffff" text="#000000">
On 05-08-08 19:49, Prabhu Ramachandran wrote:
<blockquote cite="mid:489892B8.3090206@aero.iitb.ac.in" type="cite">Chris
Kruszynski wrote:
  <br>
  <blockquote type="cite">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.
    <br>
    <br>
The test case is easy:
    <br>
  </blockquote>
[...]
  <br>
  <br>
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.
  <br>
  <br>
  <blockquote type="cite">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).
    <br>
  </blockquote>
  <br>
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.
  <br>
  <br>
  <blockquote type="cite">The problem can actually be pretty widespread
since it affects all python classes which both contain and observe a
vtk object.
    <br>
  </blockquote>
  <br>
Indeed.
  <br>
  <br>
  <blockquote type="cite">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).
    <br>
  </blockquote>
  <br>
I'm not so sure about this.  The reason is that even with older
versions of VTK you could do this:
  <br>
  <br>
import vtk
  <br>
  <br>
def f(o, e):
  <br>
    print e
  <br>
  <br>
a = vtk.vtkObject()
  <br>
a.AddObserver('DeleteEvent', f)
  <br>
del a
  <br>
  <br>
This prints "DeleteEvent", which means that 'a' was indeed deleted.  So
it wasn't as if there was a memory leak.
  <br>
  <br>
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?
  <br>
  <br>
If you notice, the crash is always in Py_Finalize() the documentation
for which has a nice section on this:
  <br>
  <br>
"""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. """
  <br>
  <br>
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.
  <br>
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.
  <br>
  <br>
Anyway, I'll check in your fix shortly.
  <br>
</blockquote>
I have dug deeper into why the crashes occur while they shouldn't, and
I have uncovered the cause: the vtkPythonHash is deleted from
vtkPythonHashDelete(), which is a Py_AtExit callback. The documentation
for Py_AtExit is very clear:<br>
<br>
"Since Python's internal finalization will have completed before the
cleanup function, no Python APIs should be called by <var>func</var>.
"<br>
<br>
But what happens if python doesn't delete a cycle upon finalizing? The
vtk object remains in the hash, and is deleted in the Py_AtExit
callback. Now if the vtk object has a reference to python objects
(callbacks), it is possible that during destruction Python API calls
are used, which can lead to the crashes we see.<br>
<br>
This affects both vtkPythonCommand as well as
vtkPythonVoidFunc/vtkPythonVoidFuncArgDelete, which is I guess what
Charl was talking about.<br>
<br>
Python does seem to support garbage collection of cyclic references
with a whole bunch of extra API calls, but this might require quite
some effort to implement, and I
don't think it should have a terribly high priority.<br>
<br>
I have attached a fix for Charl's vtkPythonVoidFunc, the test case is:<br>
<br>
import vtk
<br>
<br>
class TestCase2 :
<br>
 def __init__(self) :
<br>
   self.Filter = vtk.vtkProgrammableFilter()
<br>
   self.Filter.SetExecuteMethod(self.ExecMethod)<br>
 def ExecMethod(self) :<br>
   print 'execute method called'
<br>
<br>
test2 = TestCase2()
<br>
<br>
This one does not get fixed by self.Cycle = self, by the way. It ought
to work for any place where SetExecuteMethod is wrapped, not just
vtkProgrammableFilter, since they all use the same code.<br>
<br>
<br>
Kind regards,<br>
<br>
- Chris<br>
</body>
</html>