From 8f657c35b978b681e6e919f08358992e1aed7dc1 Mon Sep 17 00:00:00 2001
From: Benjamin Peterson <benjamin@python.org>
Date: Tue, 4 Oct 2016 00:00:02 -0700
Subject: ensure gc tracking is off when invoking weakref callbacks (closes
#26617)
---
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 6d50a66..4cfaa54 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 e87ed71..8371fbb 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 ccde3de..e71b3f8 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
--
cgit v0.12