From ad0aab6e27e85e99a8fb28b7aaf9523e4c0b7388 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 4 Oct 2016 00:00:02 -0700 Subject: [PATCH 1/2] [3.4] bpo-26617: Ensure gc tracking is off when invoking weakref callbacks. (cherry picked from commit 8f657c35b978b681e6e919f08358992e1aed7dc1) --- Lib/test/test_weakref.py | 8 ++++++++ Misc/NEWS | 2 ++ Objects/typeobject.c | 25 +++++++++++++------------ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index 4313c1d5c85620..28f012f084f8a4 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -827,6 +827,14 @@ def test_set_callback_attribute(self): 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 88d6c70fd4a742..237cc48ac5f9ba 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,8 @@ Release date: XXXX-XX-XX Core and Builtins ----------------- +- Issue #26617: Fix crash when GC runs during weakref callbacks. + - bpo-27945: Fixed various segfaults with dict when input collections are mutated during searching, inserting or comparing. Based on patches by Duane Griffin and Tim Mitchell. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ca5355a7dafc15..2fae4d7359022f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1116,11 +1116,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; @@ -1131,30 +1126,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 From 0ce5c15e8d6a7a844693becc8de5f21f387e3134 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 17 Jul 2017 08:33:26 +0300 Subject: [PATCH 2/2] Rewrite a NEWS entry as a NEWS.d entry. --- Misc/NEWS | 2 -- .../Core and Builtins/2017-07-15-13-55-22.bpo-26617.Gh5LvN.rst | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-07-15-13-55-22.bpo-26617.Gh5LvN.rst diff --git a/Misc/NEWS b/Misc/NEWS index 237cc48ac5f9ba..88d6c70fd4a742 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,8 +10,6 @@ Release date: XXXX-XX-XX Core and Builtins ----------------- -- Issue #26617: Fix crash when GC runs during weakref callbacks. - - bpo-27945: Fixed various segfaults with dict when input collections are mutated during searching, inserting or comparing. Based on patches by Duane Griffin and Tim Mitchell. diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-07-15-13-55-22.bpo-26617.Gh5LvN.rst b/Misc/NEWS.d/next/Core and Builtins/2017-07-15-13-55-22.bpo-26617.Gh5LvN.rst new file mode 100644 index 00000000000000..c3a41396df8f53 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-07-15-13-55-22.bpo-26617.Gh5LvN.rst @@ -0,0 +1 @@ +Fix crash when GC runs during weakref callbacks.