From 73ba98cc7d5eb2561a8b3b92efb64924b0bebc94 Mon Sep 17 00:00:00 2001 From: Torsten Landschoff Date: Fri, 2 Aug 2013 00:45:15 +0200 Subject: [PATCH] Add decorator @gc_no_clear to disable tp_clear slot. This can be useful if some external objects have to be cleaned up when the Python object dies. Otherwise the cyclic garbage collector may drop the references before the cleanup has any chance to run. Includes documentation and a simple unit test. Discussion: http://article.gmane.org/gmane.comp.python.cython.devel/14986 --- Cython/Compiler/ModuleNode.py | 3 ++- Cython/Compiler/Options.py | 3 +++ Cython/Compiler/Symtab.py | 7 ++++++ Cython/Compiler/TypeSlots.py | 10 +++++++- docs/src/userguide/extension_types.rst | 37 ++++++++++++++++++++++++++++++ tests/run/gc_no_clear.pyx | 42 ++++++++++++++++++++++++++++++++++ 6 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 tests/run/gc_no_clear.pyx diff --git a/Cython/Compiler/ModuleNode.py b/Cython/Compiler/ModuleNode.py index 8ba8678..d1cc241 100644 --- a/Cython/Compiler/ModuleNode.py +++ b/Cython/Compiler/ModuleNode.py @@ -1009,7 +1009,8 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode): self.generate_dealloc_function(scope, code) if scope.needs_gc(): self.generate_traverse_function(scope, code, entry) - self.generate_clear_function(scope, code, entry) + if scope.needs_tp_clear(): + self.generate_clear_function(scope, code, entry) if scope.defines_any(["__getitem__"]): self.generate_getitem_int_function(scope, code) if scope.defines_any(["__setitem__", "__delitem__"]): diff --git a/Cython/Compiler/Options.py b/Cython/Compiler/Options.py index c81a756..0b5932f 100644 --- a/Cython/Compiler/Options.py +++ b/Cython/Compiler/Options.py @@ -96,6 +96,7 @@ directive_defaults = { 'final' : False, 'internal' : False, 'profile': False, + 'no_gc_clear': False, 'linetrace': False, 'infer_types': None, 'infer_types.verbose': False, @@ -201,6 +202,7 @@ directive_types = { 'ccall' : None, 'cclass' : None, 'returns' : type, + 'no_gc_clear': bool, 'set_initial_path': str, 'freelist': int, 'c_string_type': one_of('bytes', 'str', 'unicode'), @@ -214,6 +216,7 @@ for key, val in directive_defaults.items(): directive_scopes = { # defaults to available everywhere # 'module', 'function', 'class', 'with statement' 'final' : ('cclass', 'function'), + 'no_gc_clear' : ('cclass',), 'internal' : ('cclass',), 'autotestdict' : ('module',), 'autotestdict.all' : ('module',), diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py index a1bc497..c5b2725 100644 --- a/Cython/Compiler/Symtab.py +++ b/Cython/Compiler/Symtab.py @@ -1804,6 +1804,13 @@ class CClassScope(ClassScope): return not self.parent_type.is_gc_simple return False + def needs_tp_clear(self): + """ + Do we need to generate an implementation for the tp_clear slot? Can + be disabled to keep references for the __dealloc__ cleanup function. + """ + return self.needs_gc() and not self.directives.get('no_gc_clear', False) + def declare_var(self, name, type, pos, cname = None, visibility = 'private', api = 0, in_pxd = 0, is_cdef = 0): diff --git a/Cython/Compiler/TypeSlots.py b/Cython/Compiler/TypeSlots.py index 49eacd4..2140ae6 100644 --- a/Cython/Compiler/TypeSlots.py +++ b/Cython/Compiler/TypeSlots.py @@ -331,6 +331,14 @@ class GCDependentSlot(InternalMethodSlot): return InternalMethodSlot.slot_code(self, scope) +class GCClearReferencesSlot(GCDependentSlot): + + def slot_code(self, scope): + if scope.needs_tp_clear(): + return GCDependentSlot.slot_code(self, scope) + return "0" + + class ConstructorSlot(InternalMethodSlot): # Descriptor for tp_new and tp_dealloc. @@ -753,7 +761,7 @@ slot_table = ( DocStringSlot("tp_doc"), GCDependentSlot("tp_traverse"), - GCDependentSlot("tp_clear"), + GCClearReferencesSlot("tp_clear"), # Later -- synthesize a method to split into separate ops? MethodSlot(richcmpfunc, "tp_richcompare", "__richcmp__", inherited=False), # Py3 checks for __hash__ diff --git a/docs/src/userguide/extension_types.rst b/docs/src/userguide/extension_types.rst index 5d90aea..a28a239 100644 --- a/docs/src/userguide/extension_types.rst +++ b/docs/src/userguide/extension_types.rst @@ -469,6 +469,43 @@ object called :attr:`__weakref__`. For example,:: cdef object __weakref__ +Controlling cyclic garbage collection in CPython +================================================ + +By default each extension type will support the cyclic garbage collector of +CPython. If any Python objects can be referenced, Cython will automatically +generate the ``tp_traverse`` and ``tp_clear`` slots. This is usually what you +want. + +There is at least one reason why this might not be what you want: If you need +to cleanup some external resources in the ``__dealloc__`` special function and +your object happened to be in a reference cycle, the garbage collector may +have triggered a call to ``tp_clear`` to drop references. This is the way that +reference cycles are broken so that the garbage can actually be reclaimed. + +In that case any object references have vanished by the time when +``__dealloc__`` is called. Now your cleanup code lost access to the objects it +has to clean up. In that case you can disable the cycle breaker ``tp_clear`` +by using the ``no_gc_clear`` decorator :: + + @cython.no_gc_clear + cdef class DBCursor: + cdef DBConnection conn + cdef DBAPI_Cursor *raw_cursor + # ... + def __dealloc__(self): + DBAPI_close_cursor(self.conn.raw_conn, self.raw_cursor) + +This example tries to close a cursor via a database connection when the Python +object is destroyed. The ``DBConnection`` object is kept alive by the reference +from ``DBCursor``. But if a cursor happens to be in a reference cycle, the +garbage collector may effectively "steal" the database connection reference, +which makes it impossible to clean up the cursor. + +Using the ``no_gc_clear`` decorator this can not happen anymore because the +references of a cursor object will not be cleared anymore. + + Public and external extension types ==================================== diff --git a/tests/run/gc_no_clear.pyx b/tests/run/gc_no_clear.pyx new file mode 100644 index 0000000..d0939a2 --- /dev/null +++ b/tests/run/gc_no_clear.pyx @@ -0,0 +1,42 @@ +""" +Check that the @cython.no_gc_clear decorator disables generation of the +tp_clear slot so that __dealloc__ will still see the original reference +contents. + +Discussed here: http://article.gmane.org/gmane.comp.python.cython.devel/14986 +""" + +cimport cython +from cpython.ref cimport PyObject, Py_TYPE + +# Pull tp_clear for PyTypeObject as I did not find another way to access it +# from Cython code. + +cdef extern from "Python.h": + ctypedef struct PyTypeObject: + void (*tp_clear)(object) + + +@cython.no_gc_clear +cdef class DisableTpClear: + """ + An extension type that has a tp_clear method generated to test that it + actually clears the references to NULL. + + >>> uut = DisableTpClear() + >>> uut.call_tp_clear() + >>> type(uut.requires_cleanup) + + >>> del uut + """ + + cdef public object requires_cleanup + + def __cinit__(self): + self.requires_cleanup = [ + "Some object that needs cleaning in __dealloc__"] + + cpdef public call_tp_clear(self): + cdef PyTypeObject *pto = Py_TYPE(self) + if pto.tp_clear != NULL: + pto.tp_clear(self) -- 2.7.4