(original) (raw)

changeset: 104271:c9b7272e2553 branch: 3.5 parent: 104260:b24d0f274623 user: Benjamin Peterson benjamin@python.org date: Tue Oct 04 00:00:02 2016 -0700 files: Lib/test/test_weakref.py Misc/NEWS Objects/typeobject.c description: ensure gc tracking is off when invoking weakref callbacks (closes #26617) diff -r b24d0f274623 -r c9b7272e2553 Lib/test/test_weakref.py --- a/Lib/test/test_weakref.py Mon Oct 03 08:40:50 2016 -0700 +++ b/Lib/test/test_weakref.py Tue Oct 04 00:00:02 2016 -0700 @@ -845,6 +845,14 @@ 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 -r b24d0f274623 -r c9b7272e2553 Misc/NEWS --- a/Misc/NEWS Mon Oct 03 08:40:50 2016 -0700 +++ b/Misc/NEWS Tue Oct 04 00:00:02 2016 -0700 @@ -10,6 +10,8 @@ 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 -r b24d0f274623 -r c9b7272e2553 Objects/typeobject.c --- a/Objects/typeobject.c Mon Oct 03 08:40:50 2016 -0700 +++ b/Objects/typeobject.c Tue Oct 04 00:00:02 2016 -0700 @@ -1123,11 +1123,6 @@ 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 @@ has_finalizer = type->tp_finalize || type->tp_del; - /* Maybe call finalizer; exit early if resurrected */ - if (has_finalizer) + if (type->tp_finalize) { _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. */ + _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. + + 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 /benjamin@python.org