From 796564c27b8f2e32b9fbc034bbdda75f9507ca43 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 30 Jul 2013 19:59:21 +0200 Subject: Issue #18112: PEP 442 implementation (safe object finalization). --- Doc/c-api/typeobj.rst | 49 ++++ Doc/extending/newtypes.rst | 24 +- Doc/includes/typestruct.h | 7 + Doc/library/gc.rst | 29 +-- Doc/library/weakref.rst | 23 +- Doc/reference/datamodel.rst | 10 +- Doc/whatsnew/3.4.rst | 15 ++ Include/object.h | 10 + Include/objimpl.h | 34 ++- Lib/test/test_descr.py | 10 - Lib/test/test_finalization.py | 513 ++++++++++++++++++++++++++++++++++++++++++ Lib/test/test_gc.py | 18 +- Lib/test/test_generators.py | 53 +++++ Lib/test/test_io.py | 45 ++-- Lib/test/test_sys.py | 4 +- Misc/NEWS | 2 + Modules/_io/bufferedio.c | 74 +++++- Modules/_io/fileio.c | 24 +- Modules/_io/iobase.c | 92 ++++---- Modules/_io/textio.c | 37 ++- Modules/_testcapimodule.c | 80 +++++++ Modules/gcmodule.c | 156 +++++++++---- Objects/genobject.c | 103 +++------ Objects/object.c | 70 +++++- Objects/typeobject.c | 97 ++++---- 25 files changed, 1256 insertions(+), 323 deletions(-) create mode 100644 Lib/test/test_finalization.py diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index ea1a0ad..ea787de 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -465,6 +465,14 @@ type objects) *must* have the :attr:`ob_size` field. :const:`Py_TPFLAGS_HAVE_VERSION_TAG`. + .. data:: Py_TPFLAGS_HAVE_FINALIZE + + This bit is set when the :attr:`tp_finalize` slot is present in the + type structure. + + .. versionadded:: 3.4 + + .. c:member:: char* PyTypeObject.tp_doc An optional pointer to a NUL-terminated C string giving the docstring for this @@ -968,6 +976,47 @@ type objects) *must* have the :attr:`ob_size` field. This field is not inherited; it is calculated fresh by :c:func:`PyType_Ready`. +.. c:member:: destructor PyTypeObject.tp_finalize + + An optional pointer to an instance finalization function. Its signature is + :c:type:`destructor`:: + + void tp_finalize(PyObject *) + + If :attr:`tp_finalize` is set, the interpreter calls it once when + finalizing an instance. It is called either from the garbage + collector (if the instance is part of an isolated reference cycle) or + just before the object is deallocated. Either way, it is guaranteed + to be called before attempting to break reference cycles, ensuring + that it finds the object in a sane state. + + :attr:`tp_finalize` should not mutate the current exception status; + therefore, a recommended way to write a non-trivial finalizer is:: + + static void + local_finalize(PyObject *self) + { + PyObject *error_type, *error_value, *error_traceback; + + /* Save the current exception, if any. */ + PyErr_Fetch(&error_type, &error_value, &error_traceback); + + /* ... */ + + /* Restore the saved exception. */ + PyErr_Restore(error_type, error_value, error_traceback); + } + + For this field to be taken into account (even through inheritance), + you must also set the :const:`Py_TPFLAGS_HAVE_FINALIZE` flags bit. + + This field is inherited by subtypes. + + .. versionadded:: 3.4 + + .. seealso:: "Safe object finalization" (:pep:`442`) + + .. c:member:: PyObject* PyTypeObject.tp_cache Unused. Not inherited. Internal use only. diff --git a/Doc/extending/newtypes.rst b/Doc/extending/newtypes.rst index cb20bce..d5ee877 100644 --- a/Doc/extending/newtypes.rst +++ b/Doc/extending/newtypes.rst @@ -157,7 +157,8 @@ to :const:`Py_TPFLAGS_DEFAULT`. :: Py_TPFLAGS_DEFAULT, /* tp_flags */ All types should include this constant in their flags. It enables all of the -members defined by the current version of Python. +members defined until at least Python 3.3. If you need further members, +you will need to OR the corresponding flags. We provide a doc string for the type in :attr:`tp_doc`. :: @@ -928,8 +929,9 @@ Finalization and De-allocation This function is called when the reference count of the instance of your type is reduced to zero and the Python interpreter wants to reclaim it. If your type -has memory to free or other clean-up to perform, put it here. The object itself -needs to be freed here as well. Here is an example of this function:: +has memory to free or other clean-up to perform, you can put it here. The +object itself needs to be freed here as well. Here is an example of this +function:: static void newdatatype_dealloc(newdatatypeobject * obj) @@ -981,6 +983,22 @@ done. This can be done using the :c:func:`PyErr_Fetch` and Py_TYPE(obj)->tp_free((PyObject*)self); } +.. note:: + There are limitations to what you can safely do in a deallocator function. + First, if your type supports garbage collection (using :attr:`tp_traverse` + and/or :attr:`tp_clear`), some of the object's members can have been + cleared or finalized by the time :attr:`tp_dealloc` is called. Second, in + :attr:`tp_dealloc`, your object is in an unstable state: its reference + count is equal to zero. Any call to a non-trivial object or API (as in the + example above) might end up calling :attr:`tp_dealloc` again, causing a + double free and a crash. + + Starting with Python 3.4, it is recommended not to put any complex + finalization code in :attr:`tp_dealloc`, and instead use the new + :c:member:`~PyTypeObject.tp_finalize` type method. + + .. seealso:: + :pep:`442` explains the new finalization scheme. .. index:: single: string; object representation diff --git a/Doc/includes/typestruct.h b/Doc/includes/typestruct.h index 32647c0..fcb846a 100644 --- a/Doc/includes/typestruct.h +++ b/Doc/includes/typestruct.h @@ -70,4 +70,11 @@ typedef struct _typeobject { PyObject *tp_subclasses; PyObject *tp_weaklist; + destructor tp_del; + + /* Type attribute cache version tag. Added in version 2.6 */ + unsigned int tp_version_tag; + + destructor tp_finalize; + } PyTypeObject; diff --git a/Doc/library/gc.rst b/Doc/library/gc.rst index 95df2f8..c578690 100644 --- a/Doc/library/gc.rst +++ b/Doc/library/gc.rst @@ -176,24 +176,13 @@ values but should not rebind them): .. data:: garbage - A list of objects which the collector found to be unreachable but could not be - freed (uncollectable objects). By default, this list contains only objects with - :meth:`__del__` methods. Objects that have :meth:`__del__` methods and are - part of a reference cycle cause the entire reference cycle to be uncollectable, - including objects not necessarily in the cycle but reachable only from it. - Python doesn't collect such cycles automatically because, in general, it isn't - possible for Python to guess a safe order in which to run the :meth:`__del__` - methods. If you know a safe order, you can force the issue by examining the - *garbage* list, and explicitly breaking cycles due to your objects within the - list. Note that these objects are kept alive even so by virtue of being in the - *garbage* list, so they should be removed from *garbage* too. For example, - after breaking cycles, do ``del gc.garbage[:]`` to empty the list. It's - generally better to avoid the issue by not creating cycles containing objects - with :meth:`__del__` methods, and *garbage* can be examined in that case to - verify that no such cycles are being created. - - If :const:`DEBUG_SAVEALL` is set, then all unreachable objects will be added - to this list rather than freed. + A list of objects which the collector found to be unreachable but could + not be freed (uncollectable objects). Starting with Python 3.4, this + list should be empty most of the time, except when using instances of + C extension types with a non-NULL ``tp_del`` slot. + + If :const:`DEBUG_SAVEALL` is set, then all unreachable objects will be + added to this list rather than freed. .. versionchanged:: 3.2 If this list is non-empty at interpreter shutdown, a @@ -201,6 +190,10 @@ values but should not rebind them): :const:`DEBUG_UNCOLLECTABLE` is set, in addition all uncollectable objects are printed. + .. versionchanged:: 3.4 + Following :pep:`442`, objects with a :meth:`__del__` method don't end + up in :attr:`gc.garbage` anymore. + .. data:: callbacks A list of callbacks that will be invoked by the garbage collector before and diff --git a/Doc/library/weakref.rst b/Doc/library/weakref.rst index 5b5e460..818a6fa 100644 --- a/Doc/library/weakref.rst +++ b/Doc/library/weakref.rst @@ -529,22 +529,13 @@ follows:: def __del__(self): self.remove() -This solution has a couple of serious problems: - -* There is no guarantee that the object will be garbage collected - before the program exists, so the directory might be left. This is - because reference cycles containing an object with a :meth:`__del__` - method can never be collected. And even if the :class:`TempDir` - object is not itself part of a reference cycle, it may still be kept - alive by some unkown uncollectable reference cycle. - -* The :meth:`__del__` method may be called at shutdown after the - :mod:`shutil` module has been cleaned up, in which case - :attr:`shutil.rmtree` will have been replaced by :const:`None`. - This will cause the :meth:`__del__` method to fail and the directory - will not be removed. - -Using finalizers we can avoid these problems:: +This solution has a serious problem: the :meth:`__del__` method may be +called at shutdown after the :mod:`shutil` module has been cleaned up, +in which case :attr:`shutil.rmtree` will have been replaced by :const:`None`. +This will cause the :meth:`__del__` method to fail and the directory +will not be removed. + +Using finalizers we can avoid this problem:: class TempDir: def __init__(self): diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index 1a48c1f..95028c2 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -1120,12 +1120,10 @@ Basic customization ``sys.last_traceback`` keeps the stack frame alive). The first situation can only be remedied by explicitly breaking the cycles; the latter two situations can be resolved by storing ``None`` in ``sys.last_traceback``. - Circular references which are garbage are detected when the option cycle - detector is enabled (it's on by default), but can only be cleaned up if - there are no Python- level :meth:`__del__` methods involved. Refer to the - documentation for the :mod:`gc` module for more information about how - :meth:`__del__` methods are handled by the cycle detector, particularly - the description of the ``garbage`` value. + Circular references which are garbage are detected and cleaned up when + the cyclic garbage collector is enabled (it's on by default). Refer to the + documentation for the :mod:`gc` module for more information about this + topic. .. warning:: diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst index b5be568..17cec3f 100644 --- a/Doc/whatsnew/3.4.rst +++ b/Doc/whatsnew/3.4.rst @@ -119,6 +119,21 @@ The :pep:`445` adds new Application Programming Interfaces (API) to customize Python memory allocators. +.. _pep-442: + +PEP 442: Safe object finalization +================================= + +This PEP removes the current limitations and quirks of object finalization. +With it, objects with :meth:`__del__` methods, as well as generators +with :keyword:`finally` clauses, can be finalized when they are part of a +reference cycle. + +.. seealso:: + + :pep:`442` - Safe object finalization + PEP written and implemented by Antoine Pitrou + Other Language Changes ====================== diff --git a/Include/object.h b/Include/object.h index 221b4a2..25b96e8 100644 --- a/Include/object.h +++ b/Include/object.h @@ -408,6 +408,8 @@ typedef struct _typeobject { /* Type attribute cache version tag. Added in version 2.6 */ unsigned int tp_version_tag; + destructor tp_finalize; + #ifdef COUNT_ALLOCS /* these must be last and never explicitly initialized */ Py_ssize_t tp_allocs; @@ -529,6 +531,8 @@ PyAPI_FUNC(int) PyObject_Not(PyObject *); PyAPI_FUNC(int) PyCallable_Check(PyObject *); PyAPI_FUNC(void) PyObject_ClearWeakRefs(PyObject *); +PyAPI_FUNC(void) PyObject_CallFinalizer(PyObject *); +PyAPI_FUNC(int) PyObject_CallFinalizerFromDealloc(PyObject *); /* Same as PyObject_Generic{Get,Set}Attr, but passing the attributes dict as the last parameter. */ @@ -646,6 +650,12 @@ given type object has a specified feature. Py_TPFLAGS_HAVE_VERSION_TAG | \ 0) +/* NOTE: The following flags reuse lower bits (removed as part of the + * Python 3.0 transition). */ + +/* Type structure has tp_finalize member (3.4) */ +#define Py_TPFLAGS_HAVE_FINALIZE (1UL << 0) + #ifdef Py_LIMITED_API #define PyType_HasFeature(t,f) ((PyType_GetFlags(t) & (f)) != 0) #else diff --git a/Include/objimpl.h b/Include/objimpl.h index 4e72a46..264c120 100644 --- a/Include/objimpl.h +++ b/Include/objimpl.h @@ -256,6 +256,30 @@ extern PyGC_Head *_PyGC_generation0; #define _Py_AS_GC(o) ((PyGC_Head *)(o)-1) +/* Bit 0 is set when tp_finalize is called */ +#define _PyGC_REFS_MASK_FINALIZED (1 << 0) +/* The (N-1) most significant bits contain the gc state / refcount */ +#define _PyGC_REFS_SHIFT (1) +#define _PyGC_REFS_MASK (((size_t) -1) << _PyGC_REFS_SHIFT) + +#define _PyGCHead_REFS(g) ((g)->gc.gc_refs >> _PyGC_REFS_SHIFT) +#define _PyGCHead_SET_REFS(g, v) do { \ + (g)->gc.gc_refs = ((g)->gc.gc_refs & ~_PyGC_REFS_MASK) \ + | (v << _PyGC_REFS_SHIFT); \ + } while (0) +#define _PyGCHead_DECREF(g) ((g)->gc.gc_refs -= 1 << _PyGC_REFS_SHIFT) + +#define _PyGCHead_FINALIZED(g) (((g)->gc.gc_refs & _PyGC_REFS_MASK_FINALIZED) != 0) +#define _PyGCHead_SET_FINALIZED(g, v) do { \ + (g)->gc.gc_refs = ((g)->gc.gc_refs & ~_PyGC_REFS_MASK_FINALIZED) \ + | (v != 0); \ + } while (0) + +#define _PyGC_FINALIZED(o) _PyGCHead_FINALIZED(_Py_AS_GC(o)) +#define _PyGC_SET_FINALIZED(o, v) _PyGCHead_SET_FINALIZED(_Py_AS_GC(o), v) + +#define _PyGC_REFS(o) _PyGCHead_REFS(_Py_AS_GC(o)) + #define _PyGC_REFS_UNTRACKED (-2) #define _PyGC_REFS_REACHABLE (-3) #define _PyGC_REFS_TENTATIVELY_UNREACHABLE (-4) @@ -264,9 +288,9 @@ extern PyGC_Head *_PyGC_generation0; * collector it must be safe to call the ob_traverse method. */ #define _PyObject_GC_TRACK(o) do { \ PyGC_Head *g = _Py_AS_GC(o); \ - if (g->gc.gc_refs != _PyGC_REFS_UNTRACKED) \ + if (_PyGCHead_REFS(g) != _PyGC_REFS_UNTRACKED) \ Py_FatalError("GC object already tracked"); \ - g->gc.gc_refs = _PyGC_REFS_REACHABLE; \ + _PyGCHead_SET_REFS(g, _PyGC_REFS_REACHABLE); \ g->gc.gc_next = _PyGC_generation0; \ g->gc.gc_prev = _PyGC_generation0->gc.gc_prev; \ g->gc.gc_prev->gc.gc_next = g; \ @@ -279,8 +303,8 @@ extern PyGC_Head *_PyGC_generation0; */ #define _PyObject_GC_UNTRACK(o) do { \ PyGC_Head *g = _Py_AS_GC(o); \ - assert(g->gc.gc_refs != _PyGC_REFS_UNTRACKED); \ - g->gc.gc_refs = _PyGC_REFS_UNTRACKED; \ + assert(_PyGCHead_REFS(g) != _PyGC_REFS_UNTRACKED); \ + _PyGCHead_SET_REFS(g, _PyGC_REFS_UNTRACKED); \ g->gc.gc_prev->gc.gc_next = g->gc.gc_next; \ g->gc.gc_next->gc.gc_prev = g->gc.gc_prev; \ g->gc.gc_next = NULL; \ @@ -288,7 +312,7 @@ extern PyGC_Head *_PyGC_generation0; /* True if the object is currently tracked by the GC. */ #define _PyObject_GC_IS_TRACKED(o) \ - ((_Py_AS_GC(o))->gc.gc_refs != _PyGC_REFS_UNTRACKED) + (_PyGC_REFS(o) != _PyGC_REFS_UNTRACKED) /* True if the object may be tracked by the GC in the future, or already is. This can be useful to implement some optimizations. */ diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 8cb3155..f0d6d1c 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -3736,18 +3736,8 @@ order (MRO) for bases """ # bug). del c - # If that didn't blow up, it's also interesting to see whether clearing - # the last container slot works: that will attempt to delete c again, - # which will cause c to get appended back to the container again - # "during" the del. (On non-CPython implementations, however, __del__ - # is typically not called again.) support.gc_collect() self.assertEqual(len(C.container), 1) - del C.container[-1] - if support.check_impl_detail(): - support.gc_collect() - self.assertEqual(len(C.container), 1) - self.assertEqual(C.container[-1].attr, 42) # Make c mortal again, so that the test framework with -l doesn't report # it as a leak. diff --git a/Lib/test/test_finalization.py b/Lib/test/test_finalization.py new file mode 100644 index 0000000..80c9b87 --- /dev/null +++ b/Lib/test/test_finalization.py @@ -0,0 +1,513 @@ +""" +Tests for object finalization semantics, as outlined in PEP 442. +""" + +import contextlib +import gc +import unittest +import weakref + +import _testcapi +from test import support + + +class NonGCSimpleBase: + """ + The base class for all the objects under test, equipped with various + testing features. + """ + + survivors = [] + del_calls = [] + tp_del_calls = [] + errors = [] + + _cleaning = False + + __slots__ = () + + @classmethod + def _cleanup(cls): + cls.survivors.clear() + cls.errors.clear() + gc.garbage.clear() + gc.collect() + cls.del_calls.clear() + cls.tp_del_calls.clear() + + @classmethod + @contextlib.contextmanager + def test(cls): + """ + A context manager to use around all finalization tests. + """ + with support.disable_gc(): + cls.del_calls.clear() + cls.tp_del_calls.clear() + NonGCSimpleBase._cleaning = False + try: + yield + if cls.errors: + raise cls.errors[0] + finally: + NonGCSimpleBase._cleaning = True + cls._cleanup() + + def check_sanity(self): + """ + Check the object is sane (non-broken). + """ + + def __del__(self): + """ + PEP 442 finalizer. Record that this was called, check the + object is in a sane state, and invoke a side effect. + """ + try: + if not self._cleaning: + self.del_calls.append(id(self)) + self.check_sanity() + self.side_effect() + except Exception as e: + self.errors.append(e) + + def side_effect(self): + """ + A side effect called on destruction. + """ + + +class SimpleBase(NonGCSimpleBase): + + def __init__(self): + self.id_ = id(self) + + def check_sanity(self): + assert self.id_ == id(self) + + +class NonGC(NonGCSimpleBase): + __slots__ = () + +class NonGCResurrector(NonGCSimpleBase): + __slots__ = () + + def side_effect(self): + """ + Resurrect self by storing self in a class-wide list. + """ + self.survivors.append(self) + +class Simple(SimpleBase): + pass + +class SimpleResurrector(NonGCResurrector, SimpleBase): + pass + + +class TestBase: + + def setUp(self): + self.old_garbage = gc.garbage[:] + gc.garbage[:] = [] + + def tearDown(self): + # None of the tests here should put anything in gc.garbage + try: + self.assertEqual(gc.garbage, []) + finally: + del self.old_garbage + gc.collect() + + def assert_del_calls(self, ids): + self.assertEqual(sorted(SimpleBase.del_calls), sorted(ids)) + + def assert_tp_del_calls(self, ids): + self.assertEqual(sorted(SimpleBase.tp_del_calls), sorted(ids)) + + def assert_survivors(self, ids): + self.assertEqual(sorted(id(x) for x in SimpleBase.survivors), sorted(ids)) + + def assert_garbage(self, ids): + self.assertEqual(sorted(id(x) for x in gc.garbage), sorted(ids)) + + def clear_survivors(self): + SimpleBase.survivors.clear() + + +class SimpleFinalizationTest(TestBase, unittest.TestCase): + """ + Test finalization without refcycles. + """ + + def test_simple(self): + with SimpleBase.test(): + s = Simple() + ids = [id(s)] + wr = weakref.ref(s) + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + self.assertIs(wr(), None) + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + + def test_simple_resurrect(self): + with SimpleBase.test(): + s = SimpleResurrector() + ids = [id(s)] + wr = weakref.ref(s) + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors(ids) + self.assertIsNot(wr(), None) + self.clear_survivors() + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + self.assertIs(wr(), None) + + def test_non_gc(self): + with SimpleBase.test(): + s = NonGC() + self.assertFalse(gc.is_tracked(s)) + ids = [id(s)] + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + + def test_non_gc_resurrect(self): + with SimpleBase.test(): + s = NonGCResurrector() + self.assertFalse(gc.is_tracked(s)) + ids = [id(s)] + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors(ids) + self.clear_survivors() + gc.collect() + self.assert_del_calls(ids * 2) + self.assert_survivors(ids) + + +class SelfCycleBase: + + def __init__(self): + super().__init__() + self.ref = self + + def check_sanity(self): + super().check_sanity() + assert self.ref is self + +class SimpleSelfCycle(SelfCycleBase, Simple): + pass + +class SelfCycleResurrector(SelfCycleBase, SimpleResurrector): + pass + +class SuicidalSelfCycle(SelfCycleBase, Simple): + + def side_effect(self): + """ + Explicitly break the reference cycle. + """ + self.ref = None + + +class SelfCycleFinalizationTest(TestBase, unittest.TestCase): + """ + Test finalization of an object having a single cyclic reference to + itself. + """ + + def test_simple(self): + with SimpleBase.test(): + s = SimpleSelfCycle() + ids = [id(s)] + wr = weakref.ref(s) + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + self.assertIs(wr(), None) + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + + def test_simple_resurrect(self): + # Test that __del__ can resurrect the object being finalized. + with SimpleBase.test(): + s = SelfCycleResurrector() + ids = [id(s)] + wr = weakref.ref(s) + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors(ids) + # XXX is this desirable? + self.assertIs(wr(), None) + # When trying to destroy the object a second time, __del__ + # isn't called anymore (and the object isn't resurrected). + self.clear_survivors() + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + self.assertIs(wr(), None) + + def test_simple_suicide(self): + # Test the GC is able to deal with an object that kills its last + # reference during __del__. + with SimpleBase.test(): + s = SuicidalSelfCycle() + ids = [id(s)] + wr = weakref.ref(s) + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + self.assertIs(wr(), None) + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + self.assertIs(wr(), None) + + +class ChainedBase: + + def chain(self, left): + self.suicided = False + self.left = left + left.right = self + + def check_sanity(self): + super().check_sanity() + if self.suicided: + assert self.left is None + assert self.right is None + else: + left = self.left + if left.suicided: + assert left.right is None + else: + assert left.right is self + right = self.right + if right.suicided: + assert right.left is None + else: + assert right.left is self + +class SimpleChained(ChainedBase, Simple): + pass + +class ChainedResurrector(ChainedBase, SimpleResurrector): + pass + +class SuicidalChained(ChainedBase, Simple): + + def side_effect(self): + """ + Explicitly break the reference cycle. + """ + self.suicided = True + self.left = None + self.right = None + + +class CycleChainFinalizationTest(TestBase, unittest.TestCase): + """ + Test finalization of a cyclic chain. These tests are similar in + spirit to the self-cycle tests above, but the collectable object + graph isn't trivial anymore. + """ + + def build_chain(self, classes): + nodes = [cls() for cls in classes] + for i in range(len(nodes)): + nodes[i].chain(nodes[i-1]) + return nodes + + def check_non_resurrecting_chain(self, classes): + N = len(classes) + with SimpleBase.test(): + nodes = self.build_chain(classes) + ids = [id(s) for s in nodes] + wrs = [weakref.ref(s) for s in nodes] + del nodes + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + self.assertEqual([wr() for wr in wrs], [None] * N) + gc.collect() + self.assert_del_calls(ids) + + def check_resurrecting_chain(self, classes): + N = len(classes) + with SimpleBase.test(): + nodes = self.build_chain(classes) + N = len(nodes) + ids = [id(s) for s in nodes] + survivor_ids = [id(s) for s in nodes if isinstance(s, SimpleResurrector)] + wrs = [weakref.ref(s) for s in nodes] + del nodes + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors(survivor_ids) + # XXX desirable? + self.assertEqual([wr() for wr in wrs], [None] * N) + self.clear_survivors() + gc.collect() + self.assert_del_calls(ids) + self.assert_survivors([]) + + def test_homogenous(self): + self.check_non_resurrecting_chain([SimpleChained] * 3) + + def test_homogenous_resurrect(self): + self.check_resurrecting_chain([ChainedResurrector] * 3) + + def test_homogenous_suicidal(self): + self.check_non_resurrecting_chain([SuicidalChained] * 3) + + def test_heterogenous_suicidal_one(self): + self.check_non_resurrecting_chain([SuicidalChained, SimpleChained] * 2) + + def test_heterogenous_suicidal_two(self): + self.check_non_resurrecting_chain( + [SuicidalChained] * 2 + [SimpleChained] * 2) + + def test_heterogenous_resurrect_one(self): + self.check_resurrecting_chain([ChainedResurrector, SimpleChained] * 2) + + def test_heterogenous_resurrect_two(self): + self.check_resurrecting_chain( + [ChainedResurrector, SimpleChained, SuicidalChained] * 2) + + def test_heterogenous_resurrect_three(self): + self.check_resurrecting_chain( + [ChainedResurrector] * 2 + [SimpleChained] * 2 + [SuicidalChained] * 2) + + +# NOTE: the tp_del slot isn't automatically inherited, so we have to call +# with_tp_del() for each instantiated class. + +class LegacyBase(SimpleBase): + + def __del__(self): + try: + # Do not invoke side_effect here, since we are now exercising + # the tp_del slot. + if not self._cleaning: + self.del_calls.append(id(self)) + self.check_sanity() + except Exception as e: + self.errors.append(e) + + def __tp_del__(self): + """ + Legacy (pre-PEP 442) finalizer, mapped to a tp_del slot. + """ + try: + if not self._cleaning: + self.tp_del_calls.append(id(self)) + self.check_sanity() + self.side_effect() + except Exception as e: + self.errors.append(e) + +@_testcapi.with_tp_del +class Legacy(LegacyBase): + pass + +@_testcapi.with_tp_del +class LegacyResurrector(LegacyBase): + + def side_effect(self): + """ + Resurrect self by storing self in a class-wide list. + """ + self.survivors.append(self) + +@_testcapi.with_tp_del +class LegacySelfCycle(SelfCycleBase, LegacyBase): + pass + + +class LegacyFinalizationTest(TestBase, unittest.TestCase): + """ + Test finalization of objects with a tp_del. + """ + + def tearDown(self): + # These tests need to clean up a bit more, since they create + # uncollectable objects. + gc.garbage.clear() + gc.collect() + super().tearDown() + + def test_legacy(self): + with SimpleBase.test(): + s = Legacy() + ids = [id(s)] + wr = weakref.ref(s) + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_tp_del_calls(ids) + self.assert_survivors([]) + self.assertIs(wr(), None) + gc.collect() + self.assert_del_calls(ids) + self.assert_tp_del_calls(ids) + + def test_legacy_resurrect(self): + with SimpleBase.test(): + s = LegacyResurrector() + ids = [id(s)] + wr = weakref.ref(s) + del s + gc.collect() + self.assert_del_calls(ids) + self.assert_tp_del_calls(ids) + self.assert_survivors(ids) + # weakrefs are cleared before tp_del is called. + self.assertIs(wr(), None) + self.clear_survivors() + gc.collect() + self.assert_del_calls(ids) + self.assert_tp_del_calls(ids * 2) + self.assert_survivors(ids) + self.assertIs(wr(), None) + + def test_legacy_self_cycle(self): + # Self-cycles with legacy finalizers end up in gc.garbage. + with SimpleBase.test(): + s = LegacySelfCycle() + ids = [id(s)] + wr = weakref.ref(s) + del s + gc.collect() + self.assert_del_calls([]) + self.assert_tp_del_calls([]) + self.assert_survivors([]) + self.assert_garbage(ids) + self.assertIsNot(wr(), None) + # Break the cycle to allow collection + gc.garbage[0].ref = None + self.assert_garbage([]) + self.assertIs(wr(), None) + + +def test_main(): + support.run_unittest(__name__) + +if __name__ == "__main__": + test_main() diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 6b52e5a..e8f52a5 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1,3 +1,4 @@ +import _testcapi import unittest from test.support import (verbose, refcount_test, run_unittest, strip_python_stderr) @@ -40,6 +41,7 @@ class GC_Detector(object): # gc collects it. self.wr = weakref.ref(C1055820(666), it_happened) +@_testcapi.with_tp_del class Uncollectable(object): """Create a reference cycle with multiple __del__ methods. @@ -52,7 +54,7 @@ class Uncollectable(object): self.partner = Uncollectable(partner=self) else: self.partner = partner - def __del__(self): + def __tp_del__(self): pass ### Tests @@ -141,11 +143,12 @@ class GCTests(unittest.TestCase): del a self.assertNotEqual(gc.collect(), 0) - def test_finalizer(self): + def test_legacy_finalizer(self): # A() is uncollectable if it is part of a cycle, make sure it shows up # in gc.garbage. + @_testcapi.with_tp_del class A: - def __del__(self): pass + def __tp_del__(self): pass class B: pass a = A() @@ -165,11 +168,12 @@ class GCTests(unittest.TestCase): self.fail("didn't find obj in garbage (finalizer)") gc.garbage.remove(obj) - def test_finalizer_newclass(self): + def test_legacy_finalizer_newclass(self): # A() is uncollectable if it is part of a cycle, make sure it shows up # in gc.garbage. + @_testcapi.with_tp_del class A(object): - def __del__(self): pass + def __tp_del__(self): pass class B(object): pass a = A() @@ -570,12 +574,14 @@ class GCTests(unittest.TestCase): import subprocess code = """if 1: import gc + import _testcapi + @_testcapi.with_tp_del class X: def __init__(self, name): self.name = name def __repr__(self): return "" %% self.name - def __del__(self): + def __tp_del__(self): pass x = X('first') diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index edf3443..4e92117 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -1,3 +1,55 @@ +import gc +import sys +import unittest +import weakref + +from test import support + + +class FinalizationTest(unittest.TestCase): + + def test_frame_resurrect(self): + # A generator frame can be resurrected by a generator's finalization. + def gen(): + nonlocal frame + try: + yield + finally: + frame = sys._getframe() + + g = gen() + wr = weakref.ref(g) + next(g) + del g + support.gc_collect() + self.assertIs(wr(), None) + self.assertTrue(frame) + del frame + support.gc_collect() + + def test_refcycle(self): + # A generator caught in a refcycle gets finalized anyway. + old_garbage = gc.garbage[:] + finalized = False + def gen(): + nonlocal finalized + try: + g = yield + yield 1 + finally: + finalized = True + + g = gen() + next(g) + g.send(g) + self.assertGreater(sys.getrefcount(g), 2) + self.assertFalse(finalized) + del g + support.gc_collect() + self.assertTrue(finalized) + self.assertEqual(gc.garbage, old_garbage) + + tutorial_tests = """ Let's try a simple generator: @@ -1880,6 +1932,7 @@ __test__ = {"tut": tutorial_tests, # so this works as expected in both ways of running regrtest. def test_main(verbose=None): from test import support, test_generators + support.run_unittest(__name__) support.run_doctest(test_generators, verbose) # This part isn't needed for regrtest, but for running the test directly. diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 54c0161..c1ea6f2 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -1072,12 +1072,13 @@ class CBufferedReaderTest(BufferedReaderTest, SizeofTest): def test_garbage_collection(self): # C BufferedReader objects are collected. # The Python version has __del__, so it ends into gc.garbage instead - rawio = self.FileIO(support.TESTFN, "w+b") - f = self.tp(rawio) - f.f = f - wr = weakref.ref(f) - del f - support.gc_collect() + with support.check_warnings(('', ResourceWarning)): + rawio = self.FileIO(support.TESTFN, "w+b") + f = self.tp(rawio) + f.f = f + wr = weakref.ref(f) + del f + support.gc_collect() self.assertTrue(wr() is None, wr) def test_args_error(self): @@ -1366,13 +1367,14 @@ class CBufferedWriterTest(BufferedWriterTest, SizeofTest): # C BufferedWriter objects are collected, and collecting them flushes # all data to disk. # The Python version has __del__, so it ends into gc.garbage instead - rawio = self.FileIO(support.TESTFN, "w+b") - f = self.tp(rawio) - f.write(b"123xxx") - f.x = f - wr = weakref.ref(f) - del f - support.gc_collect() + with support.check_warnings(('', ResourceWarning)): + rawio = self.FileIO(support.TESTFN, "w+b") + f = self.tp(rawio) + f.write(b"123xxx") + f.x = f + wr = weakref.ref(f) + del f + support.gc_collect() self.assertTrue(wr() is None, wr) with self.open(support.TESTFN, "rb") as f: self.assertEqual(f.read(), b"123xxx") @@ -2607,14 +2609,15 @@ class CTextIOWrapperTest(TextIOWrapperTest): # C TextIOWrapper objects are collected, and collecting them flushes # all data to disk. # The Python version has __del__, so it ends in gc.garbage instead. - rawio = io.FileIO(support.TESTFN, "wb") - b = self.BufferedWriter(rawio) - t = self.TextIOWrapper(b, encoding="ascii") - t.write("456def") - t.x = t - wr = weakref.ref(t) - del t - support.gc_collect() + with support.check_warnings(('', ResourceWarning)): + rawio = io.FileIO(support.TESTFN, "wb") + b = self.BufferedWriter(rawio) + t = self.TextIOWrapper(b, encoding="ascii") + t.write("456def") + t.x = t + wr = weakref.ref(t) + del t + support.gc_collect() self.assertTrue(wr() is None, wr) with self.open(support.TESTFN, "rb") as f: self.assertEqual(f.read(), b"456def") diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 93b9f8c..3f093e6 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -864,11 +864,11 @@ class SizeofTest(unittest.TestCase): check((1,2,3), vsize('') + 3*self.P) # type # static type: PyTypeObject - s = vsize('P2n15Pl4Pn9Pn11PI') + s = vsize('P2n15Pl4Pn9Pn11PIP') check(int, s) # (PyTypeObject + PyNumberMethods + PyMappingMethods + # PySequenceMethods + PyBufferProcs + 4P) - s = vsize('P2n15Pl4Pn9Pn11PI') + struct.calcsize('34P 3P 10P 2P 4P') + s = vsize('P2n15Pl4Pn9Pn11PIP') + struct.calcsize('34P 3P 10P 2P 4P') # Separate block for PyDictKeysObject with 4 entries s += struct.calcsize("2nPn") + 4*struct.calcsize("n2P") # class diff --git a/Misc/NEWS b/Misc/NEWS index 818f988..c08193d 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,8 @@ What's New in Python 3.4.0 Alpha 1? Core and Builtins ----------------- +- Issue #18112: PEP 442 implementation (safe object finalization). + - Issue #18552: Check return value of PyArena_AddPyObject() in obj2ast_object(). diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 6a885cf..6fe5d58 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -190,7 +190,8 @@ PyTypeObject PyBufferedIOBase_Type = { 0, /*tp_getattro*/ 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE + | Py_TPFLAGS_HAVE_FINALIZE, /*tp_flags*/ bufferediobase_doc, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -209,6 +210,16 @@ PyTypeObject PyBufferedIOBase_Type = { 0, /* tp_init */ 0, /* tp_alloc */ 0, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; @@ -220,7 +231,7 @@ typedef struct { int detached; int readable; int writable; - int deallocating; + char finalizing; /* True if this is a vanilla Buffered object (rather than a user derived class) *and* the raw stream is a vanilla FileIO object. */ @@ -384,8 +395,8 @@ _enter_buffered_busy(buffered *self) static void buffered_dealloc(buffered *self) { - self->deallocating = 1; - if (self->ok && _PyIOBase_finalize((PyObject *) self) < 0) + self->finalizing = 1; + if (_PyIOBase_finalize((PyObject *) self) < 0) return; _PyObject_GC_UNTRACK(self); self->ok = 0; @@ -428,8 +439,6 @@ buffered_traverse(buffered *self, visitproc visit, void *arg) static int buffered_clear(buffered *self) { - if (self->ok && _PyIOBase_finalize((PyObject *) self) < 0) - return -1; self->ok = 0; Py_CLEAR(self->raw); Py_CLEAR(self->dict); @@ -508,7 +517,7 @@ buffered_close(buffered *self, PyObject *args) goto end; } - if (self->deallocating) { + if (self->finalizing) { PyObject *r = buffered_dealloc_warn(self, (PyObject *) self); if (r) Py_DECREF(r); @@ -1749,6 +1758,7 @@ static PyMethodDef bufferedreader_methods[] = { static PyMemberDef bufferedreader_members[] = { {"raw", T_OBJECT, offsetof(buffered, raw), READONLY}, + {"_finalizing", T_BOOL, offsetof(buffered, finalizing), 0}, {NULL} }; @@ -1781,7 +1791,7 @@ PyTypeObject PyBufferedReader_Type = { 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE - | Py_TPFLAGS_HAVE_GC, /*tp_flags*/ + | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_FINALIZE, /*tp_flags*/ bufferedreader_doc, /* tp_doc */ (traverseproc)buffered_traverse, /* tp_traverse */ (inquiry)buffered_clear, /* tp_clear */ @@ -1800,6 +1810,16 @@ PyTypeObject PyBufferedReader_Type = { (initproc)bufferedreader_init, /* tp_init */ 0, /* tp_alloc */ PyType_GenericNew, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; @@ -2130,6 +2150,7 @@ static PyMethodDef bufferedwriter_methods[] = { static PyMemberDef bufferedwriter_members[] = { {"raw", T_OBJECT, offsetof(buffered, raw), READONLY}, + {"_finalizing", T_BOOL, offsetof(buffered, finalizing), 0}, {NULL} }; @@ -2162,7 +2183,7 @@ PyTypeObject PyBufferedWriter_Type = { 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE - | Py_TPFLAGS_HAVE_GC, /*tp_flags*/ + | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_FINALIZE, /*tp_flags*/ bufferedwriter_doc, /* tp_doc */ (traverseproc)buffered_traverse, /* tp_traverse */ (inquiry)buffered_clear, /* tp_clear */ @@ -2181,6 +2202,16 @@ PyTypeObject PyBufferedWriter_Type = { (initproc)bufferedwriter_init, /* tp_init */ 0, /* tp_alloc */ PyType_GenericNew, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; @@ -2416,7 +2447,7 @@ PyTypeObject PyBufferedRWPair_Type = { 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE - | Py_TPFLAGS_HAVE_GC, /* tp_flags */ + | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_FINALIZE, /* tp_flags */ bufferedrwpair_doc, /* tp_doc */ (traverseproc)bufferedrwpair_traverse, /* tp_traverse */ (inquiry)bufferedrwpair_clear, /* tp_clear */ @@ -2435,6 +2466,16 @@ PyTypeObject PyBufferedRWPair_Type = { (initproc)bufferedrwpair_init, /* tp_init */ 0, /* tp_alloc */ PyType_GenericNew, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; @@ -2522,6 +2563,7 @@ static PyMethodDef bufferedrandom_methods[] = { static PyMemberDef bufferedrandom_members[] = { {"raw", T_OBJECT, offsetof(buffered, raw), READONLY}, + {"_finalizing", T_BOOL, offsetof(buffered, finalizing), 0}, {NULL} }; @@ -2554,7 +2596,7 @@ PyTypeObject PyBufferedRandom_Type = { 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE - | Py_TPFLAGS_HAVE_GC, /*tp_flags*/ + | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_FINALIZE, /*tp_flags*/ bufferedrandom_doc, /* tp_doc */ (traverseproc)buffered_traverse, /* tp_traverse */ (inquiry)buffered_clear, /* tp_clear */ @@ -2573,4 +2615,14 @@ PyTypeObject PyBufferedRandom_Type = { (initproc)bufferedrandom_init, /* tp_init */ 0, /* tp_alloc */ PyType_GenericNew, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index 5280991..e88ae87 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -51,7 +51,7 @@ typedef struct { unsigned int writable : 1; signed int seekable : 2; /* -1 means unknown */ unsigned int closefd : 1; - unsigned int deallocating: 1; + char finalizing; PyObject *weakreflist; PyObject *dict; } fileio; @@ -128,7 +128,7 @@ fileio_close(fileio *self) self->fd = -1; Py_RETURN_NONE; } - if (self->deallocating) { + if (self->finalizing) { PyObject *r = fileio_dealloc_warn(self, (PyObject *) self); if (r) Py_DECREF(r); @@ -447,7 +447,7 @@ fileio_clear(fileio *self) static void fileio_dealloc(fileio *self) { - self->deallocating = 1; + self->finalizing = 1; if (_PyIOBase_finalize((PyObject *) self) < 0) return; _PyObject_GC_UNTRACK(self); @@ -1182,6 +1182,11 @@ static PyGetSetDef fileio_getsetlist[] = { {NULL}, }; +static PyMemberDef fileio_members[] = { + {"_finalizing", T_BOOL, offsetof(fileio, finalizing), 0}, + {NULL} +}; + PyTypeObject PyFileIO_Type = { PyVarObject_HEAD_INIT(NULL, 0) "_io.FileIO", @@ -1203,7 +1208,7 @@ PyTypeObject PyFileIO_Type = { 0, /* tp_setattro */ 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE - | Py_TPFLAGS_HAVE_GC, /* tp_flags */ + | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_FINALIZE, /* tp_flags */ fileio_doc, /* tp_doc */ (traverseproc)fileio_traverse, /* tp_traverse */ (inquiry)fileio_clear, /* tp_clear */ @@ -1212,7 +1217,7 @@ PyTypeObject PyFileIO_Type = { 0, /* tp_iter */ 0, /* tp_iternext */ fileio_methods, /* tp_methods */ - 0, /* tp_members */ + fileio_members, /* tp_members */ fileio_getsetlist, /* tp_getset */ 0, /* tp_base */ 0, /* tp_dict */ @@ -1223,4 +1228,13 @@ PyTypeObject PyFileIO_Type = { PyType_GenericAlloc, /* tp_alloc */ fileio_new, /* tp_new */ PyObject_GC_Del, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index e38473a..a4f2c0e 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -196,21 +196,17 @@ iobase_close(PyObject *self, PyObject *args) /* Finalization and garbage collection support */ -int -_PyIOBase_finalize(PyObject *self) +static void +iobase_finalize(PyObject *self) { PyObject *res; - PyObject *tp, *v, *tb; - int closed = 1; - int is_zombie; + PyObject *error_type, *error_value, *error_traceback; + int closed; + _Py_IDENTIFIER(_finalizing); + + /* Save the current exception, if any. */ + PyErr_Fetch(&error_type, &error_value, &error_traceback); - /* If _PyIOBase_finalize() is called from a destructor, we need to - resurrect the object as calling close() can invoke arbitrary code. */ - is_zombie = (Py_REFCNT(self) == 0); - if (is_zombie) { - ++Py_REFCNT(self); - } - PyErr_Fetch(&tp, &v, &tb); /* If `closed` doesn't exist or can't be evaluated as bool, then the object is probably in an unusable state, so ignore. */ res = PyObject_GetAttr(self, _PyIO_str_closed); @@ -223,6 +219,10 @@ _PyIOBase_finalize(PyObject *self) PyErr_Clear(); } if (closed == 0) { + /* Signal close() that it was called as part of the object + finalization process. */ + if (_PyObject_SetAttrId(self, &PyId__finalizing, Py_True)) + PyErr_Clear(); res = PyObject_CallMethodObjArgs((PyObject *) self, _PyIO_str_close, NULL); /* Silencing I/O errors is bad, but printing spurious tracebacks is @@ -233,31 +233,25 @@ _PyIOBase_finalize(PyObject *self) else Py_DECREF(res); } - PyErr_Restore(tp, v, tb); - if (is_zombie) { - if (--Py_REFCNT(self) != 0) { - /* The object lives again. The following code is taken from - slot_tp_del in typeobject.c. */ - Py_ssize_t refcnt = Py_REFCNT(self); - _Py_NewReference(self); - Py_REFCNT(self) = refcnt; - /* If Py_REF_DEBUG, _Py_NewReference bumped _Py_RefTotal, so - * we need to undo that. */ - _Py_DEC_REFTOTAL; - /* If Py_TRACE_REFS, _Py_NewReference re-added self to the object - * chain, so no more to do there. - * If COUNT_ALLOCS, the original decref bumped tp_frees, and - * _Py_NewReference bumped tp_allocs: both of those need to be - * undone. - */ -#ifdef COUNT_ALLOCS - --Py_TYPE(self)->tp_frees; - --Py_TYPE(self)->tp_allocs; -#endif - return -1; - } + + /* Restore the saved exception. */ + PyErr_Restore(error_type, error_value, error_traceback); +} + +int +_PyIOBase_finalize(PyObject *self) +{ + int is_zombie; + + /* If _PyIOBase_finalize() is called from a destructor, we need to + resurrect the object as calling close() can invoke arbitrary code. */ + is_zombie = (Py_REFCNT(self) == 0); + if (is_zombie) + return PyObject_CallFinalizerFromDealloc(self); + else { + PyObject_CallFinalizer(self); + return 0; } - return 0; } static int @@ -270,8 +264,6 @@ iobase_traverse(iobase *self, visitproc visit, void *arg) static int iobase_clear(iobase *self) { - if (_PyIOBase_finalize((PyObject *) self) < 0) - return -1; Py_CLEAR(self->dict); return 0; } @@ -741,7 +733,7 @@ PyTypeObject PyIOBase_Type = { 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE - | Py_TPFLAGS_HAVE_GC, /*tp_flags*/ + | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_FINALIZE, /*tp_flags*/ iobase_doc, /* tp_doc */ (traverseproc)iobase_traverse, /* tp_traverse */ (inquiry)iobase_clear, /* tp_clear */ @@ -760,6 +752,16 @@ PyTypeObject PyIOBase_Type = { 0, /* tp_init */ 0, /* tp_alloc */ PyType_GenericNew, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + (destructor)iobase_finalize, /* tp_finalize */ }; @@ -905,7 +907,7 @@ PyTypeObject PyRawIOBase_Type = { 0, /*tp_getattro*/ 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_FINALIZE, /*tp_flags*/ rawiobase_doc, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -924,4 +926,14 @@ PyTypeObject PyRawIOBase_Type = { 0, /* tp_init */ 0, /* tp_alloc */ 0, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c index 283411b..5d2438d 100644 --- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -173,7 +173,8 @@ PyTypeObject PyTextIOBase_Type = { 0, /*tp_getattro*/ 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE + | Py_TPFLAGS_HAVE_FINALIZE, /*tp_flags*/ textiobase_doc, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -192,6 +193,16 @@ PyTypeObject PyTextIOBase_Type = { 0, /* tp_init */ 0, /* tp_alloc */ 0, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; @@ -691,7 +702,7 @@ typedef struct char seekable; char has_read1; char telling; - char deallocating; + char finalizing; /* Specialized encoding func (see below) */ encodefunc_t encodefunc; /* Whether or not it's the start of the stream */ @@ -1112,8 +1123,6 @@ textiowrapper_init(textio *self, PyObject *args, PyObject *kwds) static int _textiowrapper_clear(textio *self) { - if (self->ok && _PyIOBase_finalize((PyObject *) self) < 0) - return -1; self->ok = 0; Py_CLEAR(self->buffer); Py_CLEAR(self->encoding); @@ -1131,9 +1140,10 @@ _textiowrapper_clear(textio *self) static void textiowrapper_dealloc(textio *self) { - self->deallocating = 1; - if (_textiowrapper_clear(self) < 0) + self->finalizing = 1; + if (_PyIOBase_finalize((PyObject *) self) < 0) return; + _textiowrapper_clear(self); _PyObject_GC_UNTRACK(self); if (self->weakreflist != NULL) PyObject_ClearWeakRefs((PyObject *)self); @@ -2573,7 +2583,7 @@ textiowrapper_close(textio *self, PyObject *args) } else { PyObject *exc = NULL, *val, *tb; - if (self->deallocating) { + if (self->finalizing) { res = _PyObject_CallMethodId(self->buffer, &PyId__dealloc_warn, "O", self); if (res) Py_DECREF(res); @@ -2734,6 +2744,7 @@ static PyMemberDef textiowrapper_members[] = { {"encoding", T_OBJECT, offsetof(textio, encoding), READONLY}, {"buffer", T_OBJECT, offsetof(textio, buffer), READONLY}, {"line_buffering", T_BOOL, offsetof(textio, line_buffering), READONLY}, + {"_finalizing", T_BOOL, offsetof(textio, finalizing), 0}, {NULL} }; @@ -2770,7 +2781,7 @@ PyTypeObject PyTextIOWrapper_Type = { 0, /*tp_setattro*/ 0, /*tp_as_buffer*/ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE - | Py_TPFLAGS_HAVE_GC, /*tp_flags*/ + | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_FINALIZE, /*tp_flags*/ textiowrapper_doc, /* tp_doc */ (traverseproc)textiowrapper_traverse, /* tp_traverse */ (inquiry)textiowrapper_clear, /* tp_clear */ @@ -2789,4 +2800,14 @@ PyTypeObject PyTextIOWrapper_Type = { (initproc)textiowrapper_init, /* tp_init */ 0, /* tp_alloc */ PyType_GenericNew, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + 0, /* tp_finalize */ }; diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 9463227..6527a53 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2491,6 +2491,85 @@ test_pytime_object_to_timespec(PyObject *self, PyObject *args) return Py_BuildValue("Nl", _PyLong_FromTime_t(sec), nsec); } +static void +slot_tp_del(PyObject *self) +{ + _Py_IDENTIFIER(__tp_del__); + PyObject *del, *res; + PyObject *error_type, *error_value, *error_traceback; + + /* Temporarily resurrect the object. */ + assert(self->ob_refcnt == 0); + self->ob_refcnt = 1; + + /* Save the current exception, if any. */ + PyErr_Fetch(&error_type, &error_value, &error_traceback); + + /* Execute __del__ method, if any. */ + del = _PyObject_LookupSpecial(self, &PyId___tp_del__); + if (del != NULL) { + res = PyEval_CallObject(del, NULL); + if (res == NULL) + PyErr_WriteUnraisable(del); + else + Py_DECREF(res); + Py_DECREF(del); + } + + /* Restore the saved exception. */ + PyErr_Restore(error_type, error_value, error_traceback); + + /* Undo the temporary resurrection; can't use DECREF here, it would + * cause a recursive call. + */ + assert(self->ob_refcnt > 0); + if (--self->ob_refcnt == 0) + return; /* this is the normal path out */ + + /* __del__ resurrected it! Make it look like the original Py_DECREF + * never happened. + */ + { + Py_ssize_t refcnt = self->ob_refcnt; + _Py_NewReference(self); + self->ob_refcnt = refcnt; + } + assert(!PyType_IS_GC(Py_TYPE(self)) || + _Py_AS_GC(self)->gc.gc_refs != _PyGC_REFS_UNTRACKED); + /* If Py_REF_DEBUG, _Py_NewReference bumped _Py_RefTotal, so + * we need to undo that. */ + _Py_DEC_REFTOTAL; + /* If Py_TRACE_REFS, _Py_NewReference re-added self to the object + * chain, so no more to do there. + * If COUNT_ALLOCS, the original decref bumped tp_frees, and + * _Py_NewReference bumped tp_allocs: both of those need to be + * undone. + */ +#ifdef COUNT_ALLOCS + --Py_TYPE(self)->tp_frees; + --Py_TYPE(self)->tp_allocs; +#endif +} + +static PyObject * +with_tp_del(PyObject *self, PyObject *args) +{ + PyObject *obj; + PyTypeObject *tp; + + if (!PyArg_ParseTuple(args, "O:with_tp_del", &obj)) + return NULL; + tp = (PyTypeObject *) obj; + if (!PyType_Check(obj) || !PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE)) { + PyErr_Format(PyExc_TypeError, + "heap type expected, got %R", obj); + return NULL; + } + tp->tp_del = slot_tp_del; + Py_INCREF(obj); + return obj; +} + static PyObject * _test_incref(PyObject *ob) { @@ -2789,6 +2868,7 @@ static PyMethodDef TestMethods[] = { {"pytime_object_to_time_t", test_pytime_object_to_time_t, METH_VARARGS}, {"pytime_object_to_timeval", test_pytime_object_to_timeval, METH_VARARGS}, {"pytime_object_to_timespec", test_pytime_object_to_timespec, METH_VARARGS}, + {"with_tp_del", with_tp_del, METH_VARARGS}, {"test_pymem", (PyCFunction)test_pymem_alloc0, METH_NOARGS}, {"test_pymem_alloc0", diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index d96d2c7..dac7feb 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -223,10 +223,10 @@ GC_TENTATIVELY_UNREACHABLE #define GC_REACHABLE _PyGC_REFS_REACHABLE #define GC_TENTATIVELY_UNREACHABLE _PyGC_REFS_TENTATIVELY_UNREACHABLE -#define IS_TRACKED(o) ((AS_GC(o))->gc.gc_refs != GC_UNTRACKED) -#define IS_REACHABLE(o) ((AS_GC(o))->gc.gc_refs == GC_REACHABLE) +#define IS_TRACKED(o) (_PyGC_REFS(o) != GC_UNTRACKED) +#define IS_REACHABLE(o) (_PyGC_REFS(o) == GC_REACHABLE) #define IS_TENTATIVELY_UNREACHABLE(o) ( \ - (AS_GC(o))->gc.gc_refs == GC_TENTATIVELY_UNREACHABLE) + _PyGC_REFS(o) == GC_TENTATIVELY_UNREACHABLE) /*** list functions ***/ @@ -341,8 +341,8 @@ update_refs(PyGC_Head *containers) { PyGC_Head *gc = containers->gc.gc_next; for (; gc != containers; gc = gc->gc.gc_next) { - assert(gc->gc.gc_refs == GC_REACHABLE); - gc->gc.gc_refs = Py_REFCNT(FROM_GC(gc)); + assert(_PyGCHead_REFS(gc) == GC_REACHABLE); + _PyGCHead_SET_REFS(gc, Py_REFCNT(FROM_GC(gc))); /* Python's cyclic gc should never see an incoming refcount * of 0: if something decref'ed to 0, it should have been * deallocated immediately at that time. @@ -361,7 +361,7 @@ update_refs(PyGC_Head *containers) * so serious that maybe this should be a release-build * check instead of an assert? */ - assert(gc->gc.gc_refs != 0); + assert(_PyGCHead_REFS(gc) != 0); } } @@ -376,9 +376,9 @@ visit_decref(PyObject *op, void *data) * generation being collected, which can be recognized * because only they have positive gc_refs. */ - assert(gc->gc.gc_refs != 0); /* else refcount was too small */ - if (gc->gc.gc_refs > 0) - gc->gc.gc_refs--; + assert(_PyGCHead_REFS(gc) != 0); /* else refcount was too small */ + if (_PyGCHead_REFS(gc) > 0) + _PyGCHead_DECREF(gc); } return 0; } @@ -407,7 +407,7 @@ visit_reachable(PyObject *op, PyGC_Head *reachable) { if (PyObject_IS_GC(op)) { PyGC_Head *gc = AS_GC(op); - const Py_ssize_t gc_refs = gc->gc.gc_refs; + const Py_ssize_t gc_refs = _PyGCHead_REFS(gc); if (gc_refs == 0) { /* This is in move_unreachable's 'young' list, but @@ -415,7 +415,7 @@ visit_reachable(PyObject *op, PyGC_Head *reachable) * we need to do is tell move_unreachable that it's * reachable. */ - gc->gc.gc_refs = 1; + _PyGCHead_SET_REFS(gc, 1); } else if (gc_refs == GC_TENTATIVELY_UNREACHABLE) { /* This had gc_refs = 0 when move_unreachable got @@ -425,7 +425,7 @@ visit_reachable(PyObject *op, PyGC_Head *reachable) * again. */ gc_list_move(gc, reachable); - gc->gc.gc_refs = 1; + _PyGCHead_SET_REFS(gc, 1); } /* Else there's nothing to do. * If gc_refs > 0, it must be in move_unreachable's 'young' @@ -469,7 +469,7 @@ move_unreachable(PyGC_Head *young, PyGC_Head *unreachable) while (gc != young) { PyGC_Head *next; - if (gc->gc.gc_refs) { + if (_PyGCHead_REFS(gc)) { /* gc is definitely reachable from outside the * original 'young'. Mark it as such, and traverse * its pointers to find any other objects that may @@ -480,8 +480,8 @@ move_unreachable(PyGC_Head *young, PyGC_Head *unreachable) */ PyObject *op = FROM_GC(gc); traverseproc traverse = Py_TYPE(op)->tp_traverse; - assert(gc->gc.gc_refs > 0); - gc->gc.gc_refs = GC_REACHABLE; + assert(_PyGCHead_REFS(gc) > 0); + _PyGCHead_SET_REFS(gc, GC_REACHABLE); (void) traverse(op, (visitproc)visit_reachable, (void *)young); @@ -500,7 +500,7 @@ move_unreachable(PyGC_Head *young, PyGC_Head *unreachable) */ next = gc->gc.gc_next; gc_list_move(gc, unreachable); - gc->gc.gc_refs = GC_TENTATIVELY_UNREACHABLE; + _PyGCHead_SET_REFS(gc, GC_TENTATIVELY_UNREACHABLE); } gc = next; } @@ -520,22 +520,19 @@ untrack_dicts(PyGC_Head *head) } } -/* Return true if object has a finalization method. */ +/* Return true if object has a pre-PEP 442 finalization method. */ static int -has_finalizer(PyObject *op) +has_legacy_finalizer(PyObject *op) { - if (PyGen_CheckExact(op)) - return PyGen_NeedsFinalizing((PyGenObject *)op); - else - return op->ob_type->tp_del != NULL; + return op->ob_type->tp_del != NULL; } -/* Move the objects in unreachable with __del__ methods into `finalizers`. +/* Move the objects in unreachable with tp_del slots into `finalizers`. * Objects moved into `finalizers` have gc_refs set to GC_REACHABLE; the * objects remaining in unreachable are left at GC_TENTATIVELY_UNREACHABLE. */ static void -move_finalizers(PyGC_Head *unreachable, PyGC_Head *finalizers) +move_legacy_finalizers(PyGC_Head *unreachable, PyGC_Head *finalizers) { PyGC_Head *gc; PyGC_Head *next; @@ -549,14 +546,14 @@ move_finalizers(PyGC_Head *unreachable, PyGC_Head *finalizers) assert(IS_TENTATIVELY_UNREACHABLE(op)); next = gc->gc.gc_next; - if (has_finalizer(op)) { + if (has_legacy_finalizer(op)) { gc_list_move(gc, finalizers); - gc->gc.gc_refs = GC_REACHABLE; + _PyGCHead_SET_REFS(gc, GC_REACHABLE); } } } -/* A traversal callback for move_finalizer_reachable. */ +/* A traversal callback for move_legacy_finalizer_reachable. */ static int visit_move(PyObject *op, PyGC_Head *tolist) { @@ -564,7 +561,7 @@ visit_move(PyObject *op, PyGC_Head *tolist) if (IS_TENTATIVELY_UNREACHABLE(op)) { PyGC_Head *gc = AS_GC(op); gc_list_move(gc, tolist); - gc->gc.gc_refs = GC_REACHABLE; + _PyGCHead_SET_REFS(gc, GC_REACHABLE); } } return 0; @@ -574,7 +571,7 @@ visit_move(PyObject *op, PyGC_Head *tolist) * into finalizers set. */ static void -move_finalizer_reachable(PyGC_Head *finalizers) +move_legacy_finalizer_reachable(PyGC_Head *finalizers) { traverseproc traverse; PyGC_Head *gc = finalizers->gc.gc_next; @@ -747,7 +744,7 @@ debug_cycle(char *msg, PyObject *op) msg, Py_TYPE(op)->tp_name, op); } -/* Handle uncollectable garbage (cycles with finalizers, and stuff reachable +/* Handle uncollectable garbage (cycles with tp_del slots, and stuff reachable * only from such cycles). * If DEBUG_SAVEALL, all objects in finalizers are appended to the module * garbage list (a Python list), else only the objects in finalizers with @@ -757,7 +754,7 @@ debug_cycle(char *msg, PyObject *op) * The finalizers list is made empty on a successful return. */ static int -handle_finalizers(PyGC_Head *finalizers, PyGC_Head *old) +handle_legacy_finalizers(PyGC_Head *finalizers, PyGC_Head *old) { PyGC_Head *gc = finalizers->gc.gc_next; @@ -769,7 +766,7 @@ handle_finalizers(PyGC_Head *finalizers, PyGC_Head *old) for (; gc != finalizers; gc = gc->gc.gc_next) { PyObject *op = FROM_GC(gc); - if ((debug & DEBUG_SAVEALL) || has_finalizer(op)) { + if ((debug & DEBUG_SAVEALL) || has_legacy_finalizer(op)) { if (PyList_Append(garbage, op) < 0) return -1; } @@ -779,6 +776,62 @@ handle_finalizers(PyGC_Head *finalizers, PyGC_Head *old) return 0; } +static void +finalize_garbage(PyGC_Head *collectable, PyGC_Head *old) +{ + destructor finalize; + PyGC_Head *gc = collectable->gc.gc_next; + + for (; gc != collectable; gc = gc->gc.gc_next) { + PyObject *op = FROM_GC(gc); + + if (!_PyGCHead_FINALIZED(gc) && + PyType_HasFeature(Py_TYPE(op), Py_TPFLAGS_HAVE_FINALIZE) && + (finalize = Py_TYPE(op)->tp_finalize) != NULL) { + _PyGCHead_SET_FINALIZED(gc, 1); + Py_INCREF(op); + finalize(op); + if (Py_REFCNT(op) == 1) { + /* op will be destroyed */ + gc = gc->gc.gc_prev; + } + Py_DECREF(op); + } + } +} + +/* Walk the collectable list and check that they are really unreachable + from the outside (some objects could have been resurrected by a + finalizer). */ +static int +check_garbage(PyGC_Head *collectable) +{ + PyGC_Head *gc; + for (gc = collectable->gc.gc_next; gc != collectable; + gc = gc->gc.gc_next) { + _PyGCHead_SET_REFS(gc, Py_REFCNT(FROM_GC(gc))); + assert(_PyGCHead_REFS(gc) != 0); + } + subtract_refs(collectable); + for (gc = collectable->gc.gc_next; gc != collectable; + gc = gc->gc.gc_next) { + assert(_PyGCHead_REFS(gc) >= 0); + if (_PyGCHead_REFS(gc) != 0) + return -1; + } + return 0; +} + +static void +revive_garbage(PyGC_Head *collectable) +{ + PyGC_Head *gc; + for (gc = collectable->gc.gc_next; gc != collectable; + gc = gc->gc.gc_next) { + _PyGCHead_SET_REFS(gc, GC_REACHABLE); + } +} + /* Break reference cycles by clearing the containers involved. This is * tricky business as the lists can be changing and we don't know which * objects may be freed. It is possible I screwed something up here. @@ -792,7 +845,6 @@ delete_garbage(PyGC_Head *collectable, PyGC_Head *old) PyGC_Head *gc = collectable->gc.gc_next; PyObject *op = FROM_GC(gc); - assert(IS_TENTATIVELY_UNREACHABLE(op)); if (debug & DEBUG_SAVEALL) { PyList_Append(garbage, op); } @@ -806,7 +858,7 @@ delete_garbage(PyGC_Head *collectable, PyGC_Head *old) if (collectable->gc.gc_next == gc) { /* object is still alive, move it, it may die later */ gc_list_move(gc, old); - gc->gc.gc_refs = GC_REACHABLE; + _PyGCHead_SET_REFS(gc, GC_REACHABLE); } } } @@ -929,19 +981,15 @@ collect(int generation, Py_ssize_t *n_collected, Py_ssize_t *n_uncollectable, } /* All objects in unreachable are trash, but objects reachable from - * finalizers can't safely be deleted. Python programmers should take - * care not to create such things. For Python, finalizers means - * instance objects with __del__ methods. Weakrefs with callbacks - * can also call arbitrary Python code but they will be dealt with by - * handle_weakrefs(). + * legacy finalizers (e.g. tp_del) can't safely be deleted. */ gc_list_init(&finalizers); - move_finalizers(&unreachable, &finalizers); - /* finalizers contains the unreachable objects with a finalizer; + move_legacy_finalizers(&unreachable, &finalizers); + /* finalizers contains the unreachable objects with a legacy finalizer; * unreachable objects reachable *from* those are also uncollectable, * and we move those into the finalizers list too. */ - move_finalizer_reachable(&finalizers); + move_legacy_finalizer_reachable(&finalizers); /* Collect statistics on collectable objects found and print * debugging information. @@ -957,11 +1005,20 @@ collect(int generation, Py_ssize_t *n_collected, Py_ssize_t *n_uncollectable, /* Clear weakrefs and invoke callbacks as necessary. */ m += handle_weakrefs(&unreachable, old); - /* Call tp_clear on objects in the unreachable set. This will cause - * the reference cycles to be broken. It may also cause some objects - * in finalizers to be freed. - */ - delete_garbage(&unreachable, old); + /* Call tp_finalize on objects which have one. */ + finalize_garbage(&unreachable, old); + + if (check_garbage(&unreachable)) { + revive_garbage(&unreachable); + gc_list_merge(&unreachable, old); + } + else { + /* Call tp_clear on objects in the unreachable set. This will cause + * the reference cycles to be broken. It may also cause some objects + * in finalizers to be freed. + */ + delete_garbage(&unreachable, old); + } /* Collect statistics on uncollectable objects found and print * debugging information. */ @@ -992,7 +1049,7 @@ collect(int generation, Py_ssize_t *n_collected, Py_ssize_t *n_uncollectable, * reachable list of garbage. The programmer has to deal with * this if they insist on creating this type of structure. */ - (void)handle_finalizers(&finalizers, old); + (void)handle_legacy_finalizers(&finalizers, old); /* Clear free list only during the collection of the highest * generation */ @@ -1662,7 +1719,8 @@ _PyObject_GC_Malloc(size_t basicsize) sizeof(PyGC_Head) + basicsize); if (g == NULL) return PyErr_NoMemory(); - g->gc.gc_refs = GC_UNTRACKED; + g->gc.gc_refs = 0; + _PyGCHead_SET_REFS(g, GC_UNTRACKED); generations[0].count++; /* number of allocated GC objects */ if (generations[0].count > generations[0].threshold && enabled && diff --git a/Objects/genobject.c b/Objects/genobject.c index 016bfa2..dfd90aa 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -16,6 +16,31 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg) } static void +gen_finalize(PyObject *self) +{ + PyGenObject *gen = (PyGenObject *)self; + PyObject *res; + PyObject *error_type, *error_value, *error_traceback; + + if (gen->gi_frame == NULL || gen->gi_frame->f_stacktop == NULL) + /* Generator isn't paused, so no need to close */ + return; + + /* Save the current exception, if any. */ + PyErr_Fetch(&error_type, &error_value, &error_traceback); + + res = gen_close(gen, NULL); + + if (res == NULL) + PyErr_WriteUnraisable(self); + else + Py_DECREF(res); + + /* Restore the saved exception. */ + PyErr_Restore(error_type, error_value, error_traceback); +} + +static void gen_dealloc(PyGenObject *gen) { PyObject *self = (PyObject *) gen; @@ -27,12 +52,8 @@ gen_dealloc(PyGenObject *gen) _PyObject_GC_TRACK(self); - if (gen->gi_frame != NULL && gen->gi_frame->f_stacktop != NULL) { - /* Generator is paused, so we need to close */ - Py_TYPE(gen)->tp_del(self); - if (self->ob_refcnt > 0) - return; /* resurrected. :( */ - } + if (PyObject_CallFinalizerFromDealloc(self)) + return; /* resurrected. :( */ _PyObject_GC_UNTRACK(self); Py_CLEAR(gen->gi_frame); @@ -40,7 +61,6 @@ gen_dealloc(PyGenObject *gen) PyObject_GC_Del(gen); } - static PyObject * gen_send_ex(PyGenObject *gen, PyObject *arg, int exc) { @@ -222,68 +242,6 @@ gen_close(PyGenObject *gen, PyObject *args) return NULL; } -static void -gen_del(PyObject *self) -{ - PyObject *res; - PyObject *error_type, *error_value, *error_traceback; - PyGenObject *gen = (PyGenObject *)self; - - if (gen->gi_frame == NULL || gen->gi_frame->f_stacktop == NULL) - /* Generator isn't paused, so no need to close */ - return; - - /* Temporarily resurrect the object. */ - assert(self->ob_refcnt == 0); - self->ob_refcnt = 1; - - /* Save the current exception, if any. */ - PyErr_Fetch(&error_type, &error_value, &error_traceback); - - res = gen_close(gen, NULL); - - if (res == NULL) - PyErr_WriteUnraisable(self); - else - Py_DECREF(res); - - /* Restore the saved exception. */ - PyErr_Restore(error_type, error_value, error_traceback); - - /* Undo the temporary resurrection; can't use DECREF here, it would - * cause a recursive call. - */ - assert(self->ob_refcnt > 0); - if (--self->ob_refcnt == 0) - return; /* this is the normal path out */ - - /* close() resurrected it! Make it look like the original Py_DECREF - * never happened. - */ - { - Py_ssize_t refcnt = self->ob_refcnt; - _Py_NewReference(self); - self->ob_refcnt = refcnt; - } - assert(PyType_IS_GC(Py_TYPE(self)) && - _Py_AS_GC(self)->gc.gc_refs != _PyGC_REFS_UNTRACKED); - - /* If Py_REF_DEBUG, _Py_NewReference bumped _Py_RefTotal, so - * we need to undo that. */ - _Py_DEC_REFTOTAL; - /* If Py_TRACE_REFS, _Py_NewReference re-added self to the object - * chain, so no more to do there. - * If COUNT_ALLOCS, the original decref bumped tp_frees, and - * _Py_NewReference bumped tp_allocs: both of those need to be - * undone. - */ -#ifdef COUNT_ALLOCS - --(Py_TYPE(self)->tp_frees); - --(Py_TYPE(self)->tp_allocs); -#endif -} - - PyDoc_STRVAR(throw_doc, "throw(typ[,val[,tb]]) -> raise exception in generator,\n\ @@ -517,7 +475,8 @@ PyTypeObject PyGen_Type = { PyObject_GenericGetAttr, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,/* tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | + Py_TPFLAGS_HAVE_FINALIZE, /* tp_flags */ 0, /* tp_doc */ (traverseproc)gen_traverse, /* tp_traverse */ 0, /* tp_clear */ @@ -544,7 +503,9 @@ PyTypeObject PyGen_Type = { 0, /* tp_cache */ 0, /* tp_subclasses */ 0, /* tp_weaklist */ - gen_del, /* tp_del */ + 0, /* tp_del */ + 0, /* tp_version_tag */ + gen_finalize, /* tp_finalize */ }; PyObject * diff --git a/Objects/object.c b/Objects/object.c index 47d3ebd..c83109d 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -255,6 +255,72 @@ _PyObject_NewVar(PyTypeObject *tp, Py_ssize_t nitems) return PyObject_INIT_VAR(op, tp, nitems); } +void +PyObject_CallFinalizer(PyObject *self) +{ + PyTypeObject *tp = Py_TYPE(self); + + /* The former could happen on heaptypes created from the C API, e.g. + PyType_FromSpec(). */ + if (!PyType_HasFeature(tp, Py_TPFLAGS_HAVE_FINALIZE) || + tp->tp_finalize == NULL) + return; + /* tp_finalize should only be called once. */ + if (PyType_IS_GC(tp) && _PyGC_FINALIZED(self)) + return; + + tp->tp_finalize(self); + if (PyType_IS_GC(tp)) + _PyGC_SET_FINALIZED(self, 1); +} + +int +PyObject_CallFinalizerFromDealloc(PyObject *self) +{ + Py_ssize_t refcnt; + + /* Temporarily resurrect the object. */ + if (self->ob_refcnt != 0) { + Py_FatalError("PyObject_CallFinalizerFromDealloc called on " + "object with a non-zero refcount"); + } + self->ob_refcnt = 1; + + PyObject_CallFinalizer(self); + + /* Undo the temporary resurrection; can't use DECREF here, it would + * cause a recursive call. + */ + assert(self->ob_refcnt > 0); + if (--self->ob_refcnt == 0) + return 0; /* this is the normal path out */ + + /* tp_finalize resurrected it! Make it look like the original Py_DECREF + * never happened. + */ + refcnt = self->ob_refcnt; + _Py_NewReference(self); + self->ob_refcnt = refcnt; + + if (PyType_IS_GC(Py_TYPE(self))) { + assert(_PyGC_REFS(self) != _PyGC_REFS_UNTRACKED); + } + /* If Py_REF_DEBUG, _Py_NewReference bumped _Py_RefTotal, so + * we need to undo that. */ + _Py_DEC_REFTOTAL; + /* If Py_TRACE_REFS, _Py_NewReference re-added self to the object + * chain, so no more to do there. + * If COUNT_ALLOCS, the original decref bumped tp_frees, and + * _Py_NewReference bumped tp_allocs: both of those need to be + * undone. + */ +#ifdef COUNT_ALLOCS + --Py_TYPE(self)->tp_frees; + --Py_TYPE(self)->tp_allocs; +#endif + return -1; +} + int PyObject_Print(PyObject *op, FILE *fp, int flags) { @@ -1981,7 +2047,7 @@ void _PyTrash_deposit_object(PyObject *op) { assert(PyObject_IS_GC(op)); - assert(_Py_AS_GC(op)->gc.gc_refs == _PyGC_REFS_UNTRACKED); + assert(_PyGC_REFS(op) == _PyGC_REFS_UNTRACKED); assert(op->ob_refcnt == 0); _Py_AS_GC(op)->gc.gc_prev = (PyGC_Head *)_PyTrash_delete_later; _PyTrash_delete_later = op; @@ -1993,7 +2059,7 @@ _PyTrash_thread_deposit_object(PyObject *op) { PyThreadState *tstate = PyThreadState_GET(); assert(PyObject_IS_GC(op)); - assert(_Py_AS_GC(op)->gc.gc_refs == _PyGC_REFS_UNTRACKED); + assert(_PyGC_REFS(op) == _PyGC_REFS_UNTRACKED); assert(op->ob_refcnt == 0); _Py_AS_GC(op)->gc.gc_prev = (PyGC_Head *) tstate->trash_delete_later; tstate->trash_delete_later = op; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b8b5076..3ff42da 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -921,6 +921,7 @@ subtype_dealloc(PyObject *self) PyTypeObject *type, *base; destructor basedealloc; PyThreadState *tstate = PyThreadState_GET(); + int has_finalizer; /* Extract the type; we expect it to be a heap type */ type = Py_TYPE(self); @@ -936,6 +937,10 @@ subtype_dealloc(PyObject *self) clear_slots(), or DECREF the dict, or clear weakrefs. */ /* Maybe call finalizer; exit early if resurrected */ + if (type->tp_finalize) { + if (PyObject_CallFinalizerFromDealloc(self) < 0) + return; + } if (type->tp_del) { type->tp_del(self); if (self->ob_refcnt > 0) @@ -987,25 +992,36 @@ subtype_dealloc(PyObject *self) assert(base); } - /* If we added a weaklist, we clear it. Do this *before* calling - the finalizer (__del__), clearing slots, or clearing the instance - dict. */ + has_finalizer = type->tp_finalize || type->tp_del; + + /* Maybe call finalizer; exit early if resurrected */ + if (has_finalizer) + _PyObject_GC_TRACK(self); + if (type->tp_finalize) { + if (PyObject_CallFinalizerFromDealloc(self) < 0) { + /* Resurrected */ + goto endlabel; + } + } + /* If we added a weaklist, we clear it. Do this *before* calling + tp_del, clearing slots, or clearing the instance dict. */ if (type->tp_weaklistoffset && !base->tp_weaklistoffset) PyObject_ClearWeakRefs(self); - /* Maybe call finalizer; exit early if resurrected */ if (type->tp_del) { - _PyObject_GC_TRACK(self); type->tp_del(self); - if (self->ob_refcnt > 0) - goto endlabel; /* resurrected */ - else - _PyObject_GC_UNTRACK(self); + if (self->ob_refcnt > 0) { + /* Resurrected */ + goto endlabel; + } + } + if (has_finalizer) { + _PyObject_GC_UNTRACK(self); /* New weakrefs could be created during the finalizer call. - If this occurs, clear them out without calling their - finalizers since they might rely on part of the object - being finalized that has already been destroyed. */ + If this occurs, clear them out without calling their + finalizers since they might rely on part of the object + being finalized that has already been destroyed. */ if (type->tp_weaklistoffset && !base->tp_weaklistoffset) { /* Modeled after GET_WEAKREFS_LISTPTR() */ PyWeakReference **list = (PyWeakReference **) \ @@ -2231,7 +2247,7 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds) /* Initialize tp_flags */ type->tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE | - Py_TPFLAGS_BASETYPE; + Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_FINALIZE; if (base->tp_flags & Py_TPFLAGS_HAVE_GC) type->tp_flags |= Py_TPFLAGS_HAVE_GC; @@ -4111,6 +4127,10 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base) COPYSLOT(tp_init); COPYSLOT(tp_alloc); COPYSLOT(tp_is_gc); + if ((type->tp_flags & Py_TPFLAGS_HAVE_FINALIZE) && + (base->tp_flags & Py_TPFLAGS_HAVE_FINALIZE)) { + COPYSLOT(tp_finalize); + } if ((type->tp_flags & Py_TPFLAGS_HAVE_GC) == (base->tp_flags & Py_TPFLAGS_HAVE_GC)) { /* They agree about gc. */ @@ -4737,6 +4757,18 @@ wrap_call(PyObject *self, PyObject *args, void *wrapped, PyObject *kwds) } static PyObject * +wrap_del(PyObject *self, PyObject *args, void *wrapped) +{ + destructor func = (destructor)wrapped; + + if (!check_num_args(args, 0)) + return NULL; + + (*func)(self); + Py_RETURN_NONE; +} + +static PyObject * wrap_richcmpfunc(PyObject *self, PyObject *args, void *wrapped, int op) { richcmpfunc func = (richcmpfunc)wrapped; @@ -5617,16 +5649,12 @@ slot_tp_new(PyTypeObject *type, PyObject *args, PyObject *kwds) } static void -slot_tp_del(PyObject *self) +slot_tp_finalize(PyObject *self) { _Py_IDENTIFIER(__del__); PyObject *del, *res; PyObject *error_type, *error_value, *error_traceback; - /* Temporarily resurrect the object. */ - assert(self->ob_refcnt == 0); - self->ob_refcnt = 1; - /* Save the current exception, if any. */ PyErr_Fetch(&error_type, &error_value, &error_traceback); @@ -5643,37 +5671,6 @@ slot_tp_del(PyObject *self) /* Restore the saved exception. */ PyErr_Restore(error_type, error_value, error_traceback); - - /* Undo the temporary resurrection; can't use DECREF here, it would - * cause a recursive call. - */ - assert(self->ob_refcnt > 0); - if (--self->ob_refcnt == 0) - return; /* this is the normal path out */ - - /* __del__ resurrected it! Make it look like the original Py_DECREF - * never happened. - */ - { - Py_ssize_t refcnt = self->ob_refcnt; - _Py_NewReference(self); - self->ob_refcnt = refcnt; - } - assert(!PyType_IS_GC(Py_TYPE(self)) || - _Py_AS_GC(self)->gc.gc_refs != _PyGC_REFS_UNTRACKED); - /* If Py_REF_DEBUG, _Py_NewReference bumped _Py_RefTotal, so - * we need to undo that. */ - _Py_DEC_REFTOTAL; - /* If Py_TRACE_REFS, _Py_NewReference re-added self to the object - * chain, so no more to do there. - * If COUNT_ALLOCS, the original decref bumped tp_frees, and - * _Py_NewReference bumped tp_allocs: both of those need to be - * undone. - */ -#ifdef COUNT_ALLOCS - --Py_TYPE(self)->tp_frees; - --Py_TYPE(self)->tp_allocs; -#endif } @@ -5782,7 +5779,7 @@ static slotdef slotdefs[] = { "see help(type(x)) for signature", PyWrapperFlag_KEYWORDS), TPSLOT("__new__", tp_new, slot_tp_new, NULL, ""), - TPSLOT("__del__", tp_del, slot_tp_del, NULL, ""), + TPSLOT("__del__", tp_finalize, slot_tp_finalize, (wrapperfunc)wrap_del, ""), BINSLOT("__add__", nb_add, slot_nb_add, "+"), -- cgit v0.12