Skip to content

Commit 41fd1aa

Browse files
[3.13] GH-91636: Clear weakrefs created by finalizers. (GH-136401)
Weakrefs to unreachable garbage that are created during running of finalizers need to be cleared. This avoids exposing objects that have `tp_clear` called on them to Python-level code. (cherry picked from commit b6b99bf) Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>
1 parent 8072d67 commit 41fd1aa

File tree

4 files changed

+54
-15
lines changed

4 files changed

+54
-15
lines changed

Lib/test/test_gc.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,11 @@ class Cyclic(tuple):
261261
# finalizer.
262262
def __del__(self):
263263
264-
# 5. Create a weakref to `func` now. If we had created
265-
# it earlier, it would have been cleared by the
266-
# garbage collector before calling the finalizers.
264+
# 5. Create a weakref to `func` now. In previous
265+
# versions of Python, this would avoid having it
266+
# cleared by the garbage collector before calling
267+
# the finalizers. Now, weakrefs get cleared after
268+
# calling finalizers.
267269
self[1].ref = weakref.ref(self[0])
268270
269271
# 6. Drop the global reference to `latefin`. The only
@@ -292,14 +294,18 @@ def func():
292294
# which will find `cyc` and `func` as garbage.
293295
gc.collect()
294296
295-
# 9. Previously, this would crash because `func_qualname`
296-
# had been NULL-ed out by func_clear().
297+
# 9. Previously, this would crash because the weakref
298+
# created in the finalizer revealed the function after
299+
# `tp_clear` was called and `func_qualname`
300+
# had been NULL-ed out by func_clear(). Now, we clear
301+
# weakrefs to unreachable objects before calling `tp_clear`
302+
# but after calling finalizers.
297303
print(f"{func=}")
298304
"""
299-
# We're mostly just checking that this doesn't crash.
300305
rc, stdout, stderr = assert_python_ok("-c", code)
301306
self.assertEqual(rc, 0)
302-
self.assertRegex(stdout, rb"""\A\s*func=<function at \S+>\s*\Z""")
307+
# The `func` global is None because the weakref was cleared.
308+
self.assertRegex(stdout, rb"""\A\s*func=None""")
303309
self.assertFalse(stderr)
304310

305311
@refcount_test
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
While performing garbage collection, clear weakrefs to unreachable objects
2+
that are created during running of finalizers. If those weakrefs were are
3+
not cleared, they could reveal unreachable objects.

Python/gc.c

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ move_legacy_finalizer_reachable(PyGC_Head *finalizers)
747747
* no object in `unreachable` is weakly referenced anymore.
748748
*/
749749
static int
750-
handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
750+
handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old, bool allow_callbacks)
751751
{
752752
PyGC_Head *gc;
753753
PyObject *op; /* generally FROM_GC(gc) */
@@ -756,7 +756,9 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
756756
PyGC_Head *next;
757757
int num_freed = 0;
758758

759-
gc_list_init(&wrcb_to_call);
759+
if (allow_callbacks) {
760+
gc_list_init(&wrcb_to_call);
761+
}
760762

761763
/* Clear all weakrefs to the objects in unreachable. If such a weakref
762764
* also has a callback, move it into `wrcb_to_call` if the callback
@@ -812,6 +814,11 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
812814
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
813815
_PyWeakref_ClearRef(wr);
814816
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
817+
818+
if (!allow_callbacks) {
819+
continue;
820+
}
821+
815822
if (wr->wr_callback == NULL) {
816823
/* no callback */
817824
continue;
@@ -864,6 +871,10 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
864871
}
865872
}
866873

874+
if (!allow_callbacks) {
875+
return 0;
876+
}
877+
867878
/* Invoke the callbacks we decided to honor. It's safe to invoke them
868879
* because they can't reference unreachable objects.
869880
*/
@@ -1399,8 +1410,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
13991410
}
14001411

14011412
/* Clear weakrefs and invoke callbacks as necessary. */
1402-
m += handle_weakrefs(&unreachable, old);
1403-
1413+
m += handle_weakrefs(&unreachable, old, true);
14041414
validate_list(old, collecting_clear_unreachable_clear);
14051415
validate_list(&unreachable, collecting_set_unreachable_clear);
14061416

@@ -1413,6 +1423,14 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
14131423
PyGC_Head final_unreachable;
14141424
handle_resurrected_objects(&unreachable, &final_unreachable, old);
14151425

1426+
/* Clear weakrefs to objects in the unreachable set. No Python-level
1427+
* code must be allowed to access those unreachable objects. During
1428+
* delete_garbage(), finalizers outside the unreachable set might run
1429+
* and create new weakrefs. If those weakrefs were not cleared, they
1430+
* could reveal unreachable objects. Callbacks are not executed.
1431+
*/
1432+
handle_weakrefs(&final_unreachable, NULL, false);
1433+
14161434
/* Call tp_clear on objects in the final_unreachable set. This will cause
14171435
* the reference cycles to be broken. It may also cause some objects
14181436
* in finalizers to be freed.

Python/gc_free_threading.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -668,9 +668,9 @@ move_legacy_finalizer_reachable(struct collection_state *state)
668668
}
669669

670670
// Clear all weakrefs to unreachable objects. Weakrefs with callbacks are
671-
// enqueued in `wrcb_to_call`, but not invoked yet.
671+
// optionally enqueued in `wrcb_to_call`, but not invoked yet.
672672
static void
673-
clear_weakrefs(struct collection_state *state)
673+
clear_weakrefs(struct collection_state *state, bool enqueue_callbacks)
674674
{
675675
PyObject *op;
676676
WORKSTACK_FOR_EACH(&state->unreachable, op) {
@@ -702,6 +702,10 @@ clear_weakrefs(struct collection_state *state)
702702
_PyWeakref_ClearRef(wr);
703703
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
704704

705+
if (!enqueue_callbacks) {
706+
continue;
707+
}
708+
705709
// We do not invoke callbacks for weakrefs that are themselves
706710
// unreachable. This is partly for historical reasons: weakrefs
707711
// predate safe object finalization, and a weakref that is itself
@@ -1154,7 +1158,7 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
11541158
interp->gc.long_lived_total = state->long_lived_total;
11551159

11561160
// Clear weakrefs and enqueue callbacks (but do not call them).
1157-
clear_weakrefs(state);
1161+
clear_weakrefs(state, true);
11581162
_PyEval_StartTheWorld(interp);
11591163

11601164
// Deallocate any object from the refcount merge step
@@ -1165,11 +1169,19 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
11651169
call_weakref_callbacks(state);
11661170
finalize_garbage(state);
11671171

1168-
// Handle any objects that may have resurrected after the finalization.
11691172
_PyEval_StopTheWorld(interp);
1173+
// Handle any objects that may have resurrected after the finalization.
11701174
err = handle_resurrected_objects(state);
11711175
// Clear free lists in all threads
11721176
_PyGC_ClearAllFreeLists(interp);
1177+
if (err == 0) {
1178+
// Clear weakrefs to objects in the unreachable set. No Python-level
1179+
// code must be allowed to access those unreachable objects. During
1180+
// delete_garbage(), finalizers outside the unreachable set might
1181+
// run and create new weakrefs. If those weakrefs were not cleared,
1182+
// they could reveal unreachable objects.
1183+
clear_weakrefs(state, false);
1184+
}
11731185
_PyEval_StartTheWorld(interp);
11741186

11751187
if (err < 0) {

0 commit comments

Comments
 (0)