On 4 August 2012 17:50, Stefan Behnel <stefan...@behnel.de> wrote: > Stefan Behnel, 04.08.2012 14:59: >> Lisandro Dalcin, 03.08.2012 18:51: >>> diff --git a/Cython/Compiler/ModuleNode.py b/Cython/Compiler/ModuleNode.py >>> index 2472de3..255b047 100644 >>> --- a/Cython/Compiler/ModuleNode.py >>> +++ b/Cython/Compiler/ModuleNode.py >>> @@ -1111,7 +1111,11 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode): >>> if base_type: >>> tp_dealloc = TypeSlots.get_base_slot_function(scope, tp_slot) >>> if tp_dealloc is None: >>> - tp_dealloc = "%s->tp_dealloc" % base_type.typeptr_cname >>> + # Using the cimported base type pointer interacts >>> + # badly with module cleanup nullyfing these pointers. >>> + # Use instead the base type pointer from the >>> + # instance's type pointer. >>> + tp_dealloc = "Py_TYPE(o)->tp_base->tp_dealloc" >>> code.putln( >>> "%s(o);" % tp_dealloc) >>> else: >> >> Tried it, doesn't work. The problem is that this always goes through the >> actual type of the object, ignoring the type hierarchy completely, which >> kills the tp_dealloc() call chain and runs into an infinite loop starting >> from the second inheritance level (or a crash because of multiple DECREF >> calls on the same fields). > > Here is a fix that should handle this correctly. > > https://github.com/cython/cython/commit/a8393fa58741c9ae0647e8fdec5fee4ffd91ddf9 > > Basically, it checks the type pointer of cimported types on tp_traverse(), > tp_clear() and tp_dealloc() and in the unlikely case that they are NULL, it > walks the type hierarchy up to the point where it finds the current > function and then calls the one in the base type. >
Nice! > It is somewhat involved, but I still doubt that it would lead to a real > slow down. Maybe tp_traverse() is the one that's a bit more time critical > than the others because it can be called more often, but the one additional > NULL check in the normal case still shouldn't hurt all that much. > OK. > It fixes the case in lxml for me, but I didn't shrink that down to a test > case yet. Lisandro, if you find the time, it would be nice if you could > write one up for the problem you ran into. > To properly test this we would need to namespace the __cleanup functions registered with atexit, I mean to name them '__<package>_<module>_cleanup', that way we could loop over registered functions to and call them "by hand". I you agree, I can take care of this and write some tests for module cleanup. -- Lisandro Dalcin --------------- CIMEC (INTEC/CONICET-UNL) Predio CONICET-Santa Fe Colectora RN 168 Km 472, Paraje El Pozo 3000 Santa Fe, Argentina Tel: +54-342-4511594 (ext 1011) Tel/Fax: +54-342-4511169 _______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
