8000 ensure gc tracking is off when invoking weakref callbacks (closes #26… · python/cpython@8f657c3 · GitHub
[go: up one dir, main page]

Skip to content

Commit 8f657c3

Browse files
committed
ensure gc tracking is off when invoking weakref callbacks (closes #26617)
1 parent b47c9d2 commit 8f657c3

File tree

3 files changed

+23
-12
lines changed

3 files changed

+23
-12
lines changed

Lib/test/test_weakref.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,14 @@ def test_set_callback_attribute(self):
845845
with self.assertRaises(AttributeError):
846846
ref1.__callback__ = lambda ref: None
847847

848+
def test_callback_gcs(self):
849+
class ObjectWithDel(Object):
850+
def __del__(self): pass
851+
x = ObjectWithDel(1)
852+
ref1 = weakref.ref(x, lambda ref: support.gc_collect())
853+
del x
854+
support.gc_collect()
855+
848856

849857
class SubclassableWeakrefTestCase(TestBase):
850858

Misc/NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ Release date: TBA
1010
Core and Builtins
1111
-----------------
1212

13+
- Issue #26617: Fix crash when GC runs during weakref callbacks.
14+
1315
- Issue #27942: String constants now interned recursively in tuples and frozensets.
1416

1517
- Issue #21578: Fixed misleading error message when ImportError called with

Objects/typeobject.c

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,11 +1123,6 @@ subtype_dealloc(PyObject *self)
11231123
Py_TRASHCAN_SAFE_BEGIN(self);
11241124
--_PyTrash_delete_nesting;
11251125
-- tstate->trash_delete_nesting;
1126-
/* DO NOT restore GC tracking at this point. weakref callbacks
1127-
* (if any, and whether directly here or indirectly in something we
1128-
* call) may trigger GC, and if self is tracked at that point, it
1129-
* will look like trash to GC and GC will try to delete self again.
1130-
*/
11311126

11321127
/* Find the nearest base with a different tp_dealloc */
11331128
base = type;
@@ -1138,30 +1133,36 @@ subtype_dealloc(PyObject *self)
11381133

11391134
has_finalizer = type->tp_finalize || type->tp_del;
11401135

1141-
/* Maybe call finalizer; exit early if resurrected */
1142-
if (has_finalizer)
1143-
_PyObject_GC_TRACK(self);
1144-
11451136
if (type->tp_finalize) {
1137+
_PyObject_GC_TRACK(self);
11461138
if (PyObject_CallFinalizerFromDealloc(self) < 0) {
11471139
/* Resurrected */
11481140
goto endlabel;
11491141
}
1142+
_PyObject_GC_UNTRACK(self);
11501143
}
1151-
/* If we added a weaklist, we clear it. Do this *before* calling
1152-
tp_del, clearing slots, or clearing the instance dict. */
1144+
/*
1145+
If we added a weaklist, we clear it. Do this *before* calling tp_del,
1146+
clearing slots, or clearing the instance dict.
1147+
1148+
GC tracking must be off at this point. weakref callbacks (if any, and
1149+
whether directly here or indirectly in something we call) may trigger GC,
1150+
and if self is tracked at that point, it will look like trash to GC and GC
1151+
will try to delete self again.
1152+
*/
11531153
if (type->tp_weaklistoffset && !base->tp_weaklistoffset)
11541154
PyObject_ClearWeakRefs(self);
11551155

11561156
if (type->tp_del) {
1157+
_PyObject_GC_TRACK(self);
11571158
type->tp_del(self);
11581159
if (self->ob_refcnt > 0) {
11591160
/* Resurrected */
11601161
goto endlabel;
11611162
}
1163+
_PyObject_GC_UNTRACK(self);
11621164
}
11631165
if (has_finalizer) {
1164-
_PyObject_GC_UNTRACK(self);
11651166
/* New weakrefs could be created during the finalizer call.
11661167
If this occurs, clear them out without calling their
11671168
finalizers since they might rely on part of the object

0 commit comments

Comments
 (0)
0