diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index 6d50a66d7ba..4cfaa548864 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -845,6 +845,14 @@ class ReferencesTestCase(TestBase): with self.assertRaises(AttributeError): ref1.__callback__ = lambda ref: None + def test_callback_gcs(self): + class ObjectWithDel(Object): + def __del__(self): pass + x = ObjectWithDel(1) + ref1 = weakref.ref(x, lambda ref: support.gc_collect()) + del x + support.gc_collect() + class SubclassableWeakrefTestCase(TestBase): diff --git a/Misc/NEWS b/Misc/NEWS index e87ed71e7a0..8371fbb39df 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,8 @@ Release date: TBA Core and Builtins ----------------- +- Issue #26617: Fix crash when GC runs during weakref callbacks. + - Issue #27942: String constants now interned recursively in tuples and frozensets. - Issue #21578: Fixed misleading error message when ImportError called with diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ccde3de7c81..e71b3f82317 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1123,11 +1123,6 @@ subtype_dealloc(PyObject *self) Py_TRASHCAN_SAFE_BEGIN(self); --_PyTrash_delete_nesting; -- tstate->trash_delete_nesting; - /* DO NOT restore GC tracking at this point. weakref callbacks - * (if any, and whether directly here or indirectly in something we - * call) may trigger GC, and if self is tracked at that point, it - * will look like trash to GC and GC will try to delete self again. - */ /* Find the nearest base with a different tp_dealloc */ base = type; @@ -1138,30 +1133,36 @@ subtype_dealloc(PyObject *self) 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) { + _PyObject_GC_TRACK(self); if (PyObject_CallFinalizerFromDealloc(self) < 0) { /* Resurrected */ goto endlabel; } + _PyObject_GC_UNTRACK(self); } - /* If we added a weaklist, we clear it. Do this *before* calling - tp_del, clearing slots, or clearing the instance dict. */ + /* + If we added a weaklist, we clear it. Do this *before* calling tp_del, + clearing slots, or clearing the instance dict. + + GC tracking must be off at this point. weakref callbacks (if any, and + whether directly here or indirectly in something we call) may trigger GC, + and if self is tracked at that point, it will look like trash to GC and GC + will try to delete self again. + */ if (type->tp_weaklistoffset && !base->tp_weaklistoffset) PyObject_ClearWeakRefs(self); if (type->tp_del) { + _PyObject_GC_TRACK(self); type->tp_del(self); if (self->ob_refcnt > 0) { /* Resurrected */ goto endlabel; } + _PyObject_GC_UNTRACK(self); } 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