From 45a17f1aa020a25b04ca85eb5f463707fae49ee6 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Tue, 6 Aug 2013 09:24:45 +0200 Subject: [PATCH] revert actual tp_finalize() usages, needs more thought --- CHANGES.rst | 4 - Cython/Compiler/ModuleNode.py | 157 ++++++++++++++------------------------- Cython/Compiler/Symtab.py | 48 +++++------- Cython/Compiler/TypeSlots.py | 20 +---- Cython/Utility/CythonFunction.c | 4 +- Cython/Utility/ExtensionTypes.c | 21 ------ Cython/Utility/Generator.c | 22 ++---- Cython/Utility/ModuleSetupCode.c | 4 - 8 files changed, 83 insertions(+), 197 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 002379a..612a9c5 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -8,10 +8,6 @@ Cython Changelog Features added -------------- -* Starting with CPython 3.4, the user provided finalisation code in the - ``__dealloc__()`` special method is called by ``tp_finalize()`` instead - of ``tp_dealloc()`` to provide a safer execution environment. - * During cyclic garbage collection, attributes of extension types that cannot create reference cycles due to their type (e.g. strings) are no longer considered for traversal or clearing. diff --git a/Cython/Compiler/ModuleNode.py b/Cython/Compiler/ModuleNode.py index 46c39c7..6f2a114 100644 --- a/Cython/Compiler/ModuleNode.py +++ b/Cython/Compiler/ModuleNode.py @@ -1006,7 +1006,6 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode): if scope: # could be None if there was an error self.generate_exttype_vtable(scope, code) self.generate_new_function(scope, code, entry) - self.generate_finalize_function(scope, code) self.generate_dealloc_function(scope, code) if scope.needs_gc(): self.generate_traverse_function(scope, code, entry) @@ -1173,40 +1172,33 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode): slot_func = scope.mangle_internal("tp_dealloc") base_type = scope.parent_type.base_type if tp_slot.slot_code(scope) != slot_func: - return # never used - - cpp_class_attrs = [entry for entry in scope.var_entries - if entry.type.is_cpp_class] - needs_finalisation = scope.needs_finalisation() - needs_gc = scope.needs_gc() + return # never used slot_func_cname = scope.mangle_internal("tp_dealloc") code.putln("") code.putln( "static void %s(PyObject *o) {" % slot_func_cname) - if cpp_class_attrs: - self.generate_self_cast(scope, code) - if needs_finalisation: - # before Py3.4, tp_finalize() isn't available, so this is - # the earliest possible time where we can call it ourselves - code.putln("#if PY_VERSION_HEX < 0x030400a1") - if needs_gc: - # We must mark ths object as (gc) untracked while tearing - # it down, lest the garbage collection is invoked while - # running this destructor. - code.putln("PyObject_GC_UnTrack(o);") - slot_func_cname = scope.mangle_internal("tp_finalize") - code.putln("%s(o);" % slot_func_cname) - if needs_gc and base_type: - # The base class deallocator probably expects this to be - # tracked, so undo the untracking above. - code.putln("PyObject_GC_Track(o);") - code.putln("#endif") + weakref_slot = scope.lookup_here("__weakref__") + _, (py_attrs, _, memoryview_slices) = scope.get_refcounted_entries() + cpp_class_attrs = [entry for entry in scope.var_entries if entry.type.is_cpp_class] - if needs_gc and not base_type: + if (py_attrs + or cpp_class_attrs + or memoryview_slices + or weakref_slot in scope.var_entries): + self.generate_self_cast(scope, code) + + # We must mark ths object as (gc) untracked while tearing it down, lest + # the garbage collection is invoked while running this destructor. + if scope.needs_gc(): code.putln("PyObject_GC_UnTrack(o);") + # call the user's __dealloc__ + self.generate_usr_dealloc_call(scope, code) + if weakref_slot in scope.var_entries: + code.putln("if (p->__weakref__) PyObject_ClearWeakRefs(o);") + for entry in cpp_class_attrs: split_cname = entry.type.cname.split('::') destructor_name = split_cname.pop() @@ -1214,10 +1206,23 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode): while destructor_name.count('<') != destructor_name.count('>'): destructor_name = split_cname.pop() + '::' + destructor_name destructor_name = destructor_name.split('<',1)[0] - code.putln("p->%s.%s::~%s();" % ( - entry.cname, entry.type.declaration_code(""), destructor_name)) + code.putln("p->%s.%s::~%s();" % + (entry.cname, entry.type.declaration_code(""), destructor_name)) + + for entry in py_attrs: + code.put_xdecref_clear("p->%s" % entry.cname, entry.type, nanny=False, + clear_before_decref=True) + + for entry in memoryview_slices: + code.put_xdecref_memoryviewslice("p->%s" % entry.cname, + have_gil=True) if base_type: + # The base class deallocator probably expects this to be tracked, so + # undo the untracking above. + if scope.needs_gc(): + code.putln("PyObject_GC_Track(o);") + tp_dealloc = TypeSlots.get_base_slot_function(scope, tp_slot) if tp_dealloc is not None: code.putln("%s(o);" % tp_dealloc) @@ -1229,8 +1234,7 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode): # the module cleanup, which may already have cleared it. # In that case, fall back to traversing the type hierarchy. base_cname = base_type.typeptr_cname - code.putln("if (likely(%s)) %s->tp_dealloc(o); " - "else __Pyx_call_next_tp_dealloc(o, %s);" % ( + code.putln("if (likely(%s)) %s->tp_dealloc(o); else __Pyx_call_next_tp_dealloc(o, %s);" % ( base_cname, base_cname, slot_func_cname)) code.globalstate.use_utility_code( UtilityCode.load_cached("CallNextTpDealloc", "ExtensionTypes.c")) @@ -1252,81 +1256,28 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode): code.putln( "}") - def generate_finalize_function(self, scope, code): - if not scope.needs_finalisation(): - return - - weakref_slot = scope.lookup_here("__weakref__") - if weakref_slot not in scope.var_entries: - weakref_slot = None - _, (py_attrs, _, memoryview_slices) = scope.get_refcounted_entries() - - slot_func_cname = scope.mangle_internal("tp_finalize") - code.putln("") - code.putln("static void %s(PyObject *o) {" % slot_func_cname) - - if py_attrs or memoryview_slices or weakref_slot: - self.generate_self_cast(scope, code) - + def generate_usr_dealloc_call(self, scope, code): entry = scope.lookup_here("__dealloc__") if entry: - code.putln("PyObject *etype, *eval, *etb;") - code.putln("PyErr_Fetch(&etype, &eval, &etb);") - - code.putln("#if PY_VERSION_HEX < 0x030400a1") - code.putln("++Py_REFCNT(o);") - code.putln("#endif") - - code.putln("%s(o);" % entry.func_cname) - code.putln("if (PyErr_Occurred()) PyErr_WriteUnraisable(o);") - - code.putln("#if PY_VERSION_HEX < 0x030400a1") - code.putln("--Py_REFCNT(o);") - code.putln("#endif") - - code.putln("PyErr_Restore(etype, eval, etb);") - - if weakref_slot: - # Not using a preprocessor test here to avoid warning about - # "unused variable p". Py3.4+ already cleared the weak refs - # when we get here. - # FIXME: so did Py<3.4 I think? Isn't this redundant? - code.putln("if ((PY_VERSION_HEX < 0x030400a1) && p->__weakref__) " - "PyObject_ClearWeakRefs(o);") - - for entry in py_attrs: - code.put_xdecref_clear("p->%s" % entry.cname, entry.type, nanny=False, - clear_before_decref=True) - - for entry in memoryview_slices: - code.put_xdecref_memoryviewslice("p->%s" % entry.cname, - have_gil=True) - - base_type = scope.parent_type.base_type - if base_type: - code.putln("#if PY_VERSION_HEX >= 0x030400a1") - # need to call base_type method - tp_finalize = TypeSlots.get_base_slot_function( - scope, TypeSlots.FinaliserSlot()) - if tp_finalize is not None: - code.putln("%s(o);" % tp_finalize) - elif base_type.is_builtin_type: - code.putln("if (%s->tp_finalize) %s->tp_finalize(o);" % ( - base_type.typeptr_cname, base_type.typeptr_cname)) - else: - # This is an externally defined type. Calling through the - # cimported base type pointer directly interacts badly with - # the module cleanup, which may already have cleared it. - # In that case, fall back to traversing the type hierarchy. - base_cname = base_type.typeptr_cname - code.putln("if (likely(%s && %s->tp_finalize)) %s->tp_finalize(o); " - "else __Pyx_call_next_tp_finalize(o, %s);" % ( - base_cname, base_cname, base_cname, slot_func_cname)) - code.globalstate.use_utility_code( - UtilityCode.load_cached("CallNextTpFinalize", "ExtensionTypes.c")) - code.putln("#endif") - - code.putln("}") + code.putln( + "{") + code.putln( + "PyObject *etype, *eval, *etb;") + code.putln( + "PyErr_Fetch(&etype, &eval, &etb);") + code.putln( + "++Py_REFCNT(o);") + code.putln( + "%s(o);" % + entry.func_cname) + code.putln( + "if (PyErr_Occurred()) PyErr_WriteUnraisable(o);") + code.putln( + "--Py_REFCNT(o);") + code.putln( + "PyErr_Restore(etype, eval, etb);") + code.putln( + "}") def generate_traverse_function(self, scope, code, cclass_entry): tp_slot = TypeSlots.GCDependentSlot("tp_traverse") diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py index 7bb019d..a1bc497 100644 --- a/Cython/Compiler/Symtab.py +++ b/Cython/Compiler/Symtab.py @@ -832,6 +832,25 @@ class Scope(object): def add_include_file(self, filename): self.outer_scope.add_include_file(filename) + def get_refcounted_entries(self, include_weakref=False, + include_gc_simple=True): + py_attrs = [] + py_buffers = [] + memoryview_slices = [] + + for entry in self.var_entries: + if entry.type.is_pyobject: + if include_weakref or entry.name != "__weakref__": + if include_gc_simple or not entry.type.is_gc_simple: + py_attrs.append(entry) + elif entry.type == PyrexTypes.c_py_buffer_type: + py_buffers.append(entry) + elif entry.type.is_memoryviewslice: + memoryview_slices.append(entry) + + have_entries = py_attrs or py_buffers or memoryview_slices + return have_entries, (py_attrs, py_buffers, memoryview_slices) + class PreImportScope(Scope): @@ -1785,33 +1804,6 @@ class CClassScope(ClassScope): return not self.parent_type.is_gc_simple return False - def get_refcounted_entries(self, include_weakref=False, - include_gc_simple=True): - py_attrs = [] - py_buffers = [] - memoryview_slices = [] - - for entry in self.var_entries: - if entry.type.is_pyobject: - if include_weakref or entry.name != "__weakref__": - if include_gc_simple or not entry.type.is_gc_simple: - py_attrs.append(entry) - elif entry.type == PyrexTypes.c_py_buffer_type: - py_buffers.append(entry) - elif entry.type.is_memoryviewslice: - memoryview_slices.append(entry) - - have_entries = py_attrs or py_buffers or memoryview_slices - return have_entries, (py_attrs, py_buffers, memoryview_slices) - - def needs_finalisation(self): - if self.lookup_here("__dealloc__"): - return True - has_gc_entries, _ = self.get_refcounted_entries(include_weakref=True) - if has_gc_entries: - return True - return False - def declare_var(self, name, type, pos, cname = None, visibility = 'private', api = 0, in_pxd = 0, is_cdef = 0): @@ -1873,6 +1865,7 @@ class CClassScope(ClassScope): self.namespace_cname = "(PyObject *)%s" % self.parent_type.typeptr_cname return entry + def declare_pyfunction(self, name, pos, allow_redefine=False): # Add an entry for a method. if name in ('__eq__', '__ne__', '__lt__', '__gt__', '__le__', '__ge__'): @@ -2040,7 +2033,6 @@ class CClassScope(ClassScope): if base_entry.utility_code: entry.utility_code = base_entry.utility_code - class CppClassScope(Scope): # Namespace of a C++ class. diff --git a/Cython/Compiler/TypeSlots.py b/Cython/Compiler/TypeSlots.py index c82c66a..e3b7a6d 100644 --- a/Cython/Compiler/TypeSlots.py +++ b/Cython/Compiler/TypeSlots.py @@ -331,21 +331,6 @@ class GCDependentSlot(InternalMethodSlot): return InternalMethodSlot.slot_code(self, scope) -class FinaliserSlot(InternalMethodSlot): - """ - Descriptor for tp_finalize(). - """ - def __init__(self): - InternalMethodSlot.__init__( - self, 'tp_finalize', - ifdef="PY_VERSION_HEX >= 0x030400a1") - - def slot_code(self, scope): - if not scope.needs_finalisation(): - return '0' - return InternalMethodSlot.slot_code(self, scope) - - class ConstructorSlot(InternalMethodSlot): # Descriptor for tp_new and tp_dealloc. @@ -405,8 +390,6 @@ class TypeFlagsSlot(SlotDescriptor): value += "|Py_TPFLAGS_BASETYPE" if scope.needs_gc(): value += "|Py_TPFLAGS_HAVE_GC" - if scope.needs_finalisation(): - value += "|Py_TPFLAGS_HAVE_FINALIZE" return value @@ -805,7 +788,8 @@ slot_table = ( EmptySlot("tp_weaklist"), EmptySlot("tp_del"), EmptySlot("tp_version_tag", ifdef="PY_VERSION_HEX >= 0x02060000"), - FinaliserSlot(), # 'tp_finalize' + # TODO: change __dealloc__ to be called by tp_finalize (PEP 442) + EmptySlot("tp_finalize", ifdef="PY_VERSION_HEX >= 0x03040a00"), ) #------------------------------------------------------------------------------------------ diff --git a/Cython/Utility/CythonFunction.c b/Cython/Utility/CythonFunction.c index 4d1b077..1d479ed 100644 --- a/Cython/Utility/CythonFunction.c +++ b/Cython/Utility/CythonFunction.c @@ -634,7 +634,7 @@ static PyTypeObject __pyx_CyFunctionType_type = { #if PY_VERSION_HEX >= 0x02060000 0, /*tp_version_tag*/ #endif -#if PY_VERSION_HEX >= 0x030400a1 +#if PY_VERSION_HEX >= 0x03040a00 0, /*tp_finalize*/ #endif }; @@ -1082,7 +1082,7 @@ static PyTypeObject __pyx_FusedFunctionType_type = { #if PY_VERSION_HEX >= 0x02060000 0, /*tp_version_tag*/ #endif -#if PY_VERSION_HEX >= 0x030400a1 +#if PY_VERSION_HEX >= 0x03040a00 0, /*tp_finalize*/ #endif }; diff --git a/Cython/Utility/ExtensionTypes.c b/Cython/Utility/ExtensionTypes.c index f1fcd84..423ed96 100644 --- a/Cython/Utility/ExtensionTypes.c +++ b/Cython/Utility/ExtensionTypes.c @@ -16,27 +16,6 @@ static void __Pyx_call_next_tp_dealloc(PyObject* obj, destructor current_tp_deal type->tp_dealloc(obj); } -/////////////// CallNextTpFinalize.proto /////////////// - -#if PY_VERSION_HEX >= 0x030400a1 -static void __Pyx_call_next_tp_finalize(PyObject* obj, destructor current_tp_finalize); -#endif - -/////////////// CallNextTpFinalize /////////////// - -#if PY_VERSION_HEX >= 0x030400a1 -static void __Pyx_call_next_tp_finalize(PyObject* obj, destructor current_tp_finalize) { - PyTypeObject* type = Py_TYPE(obj); - /* try to find the first parent type that has a different tp_finalize() function */ - while (type && type->tp_finalize != current_tp_finalize) - type = type->tp_base; - while (type && (!type->tp_finalize || type->tp_finalize == current_tp_finalize)) - type = type->tp_base; - if (type) - type->tp_finalize(obj); -} -#endif - /////////////// CallNextTpTraverse.proto /////////////// static int __Pyx_call_next_tp_traverse(PyObject* obj, visitproc v, void *a, traverseproc current_tp_traverse); diff --git a/Cython/Utility/Generator.c b/Cython/Utility/Generator.c index d1ab132..464690c 100644 --- a/Cython/Utility/Generator.c +++ b/Cython/Utility/Generator.c @@ -460,20 +460,16 @@ static void __Pyx_Generator_dealloc(PyObject *self) { PyObject_GC_UnTrack(gen); if (gen->gi_weakreflist != NULL) PyObject_ClearWeakRefs(self); + PyObject_GC_Track(self); if (gen->resume_label > 0) { /* Generator is paused, so we need to close */ - PyObject_GC_Track(self); -#if PY_VERSION_HEX >= 0x030400a1 - if (PyObject_CallFinalizerFromDealloc(self)) -#else Py_TYPE(gen)->tp_del(self); if (self->ob_refcnt > 0) -#endif return; /* resurrected. :( */ - PyObject_GC_UnTrack(self); } + PyObject_GC_UnTrack(self); __Pyx_Generator_clear(self); PyObject_GC_Del(gen); } @@ -486,11 +482,9 @@ static void __Pyx_Generator_del(PyObject *self) { if (gen->resume_label <= 0) return ; -#if PY_VERSION_HEX < 0x030400a1 /* Temporarily resurrect the object. */ assert(self->ob_refcnt == 0); self->ob_refcnt = 1; -#endif /* Save the current exception, if any. */ __Pyx_ErrFetch(&error_type, &error_value, &error_traceback); @@ -505,7 +499,6 @@ static void __Pyx_Generator_del(PyObject *self) { /* Restore the saved exception. */ __Pyx_ErrRestore(error_type, error_value, error_traceback); -#if PY_VERSION_HEX < 0x030400a1 /* Undo the temporary resurrection; can't use DECREF here, it would * cause a recursive call. */ @@ -539,7 +532,6 @@ static void __Pyx_Generator_del(PyObject *self) { --Py_TYPE(self)->tp_frees; --Py_TYPE(self)->tp_allocs; #endif -#endif } static PyMemberDef __pyx_Generator_memberlist[] = { @@ -586,7 +578,7 @@ static PyTypeObject __pyx_GeneratorType_type = { 0, /*tp_getattro*/ 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_FINALIZE, /* tp_flags*/ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags*/ 0, /*tp_doc*/ (traverseproc) __Pyx_Generator_traverse, /*tp_traverse*/ 0, /*tp_clear*/ @@ -612,16 +604,12 @@ static PyTypeObject __pyx_GeneratorType_type = { 0, /*tp_cache*/ 0, /*tp_subclasses*/ 0, /*tp_weaklist*/ -#if PY_VERSION_HEX >= 0x030400a1 - 0, -#else __Pyx_Generator_del, /*tp_del*/ -#endif #if PY_VERSION_HEX >= 0x02060000 0, /*tp_version_tag*/ #endif -#if PY_VERSION_HEX >= 0x030400a1 - __Pyx_Generator_del, /*tp_finalize*/ +#if PY_VERSION_HEX >= 0x03040a00 + 0, /*tp_finalize*/ #endif }; diff --git a/Cython/Utility/ModuleSetupCode.c b/Cython/Utility/ModuleSetupCode.c index 8065ab9..c0c59f6 100644 --- a/Cython/Utility/ModuleSetupCode.c +++ b/Cython/Utility/ModuleSetupCode.c @@ -128,10 +128,6 @@ #define Py_TPFLAGS_HAVE_VERSION_TAG 0 #endif -#if PY_VERSION_HEX < 0x030400a1 && !defined(Py_TPFLAGS_HAVE_FINALIZE) - #define Py_TPFLAGS_HAVE_FINALIZE 0 -#endif - /* new Py3.3 unicode type (PEP 393) */ #if PY_VERSION_HEX > 0x03030000 && defined(PyUnicode_KIND) #define CYTHON_PEP393_ENABLED 1 -- 2.7.4