8000 GH-91636: Clear weakrefs created by finalizers. (GH-136401) · python/cpython@b6b99bf · GitHub
[go: up one dir, main page]

Skip to content

Commit b6b99bf

Browse files
authored
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.
1 parent bc9bc07 commit b6b99bf

File tree

4 files changed

+54
-14
lines changed

4 files changed

+54
-14
lines changed

Lib/test/test_gc.py

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

306312
@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 & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ move_legacy_finalizer_reachable(PyGC_Head *finalizers)
870870
* no object in `unreachable` is weakly referenced anymore.
871871
*/
872872
static int
873-
handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
873+
handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old, bool allow_callbacks)
874874
{
875875
PyGC_Head *gc;
876876
PyObject *op; /* generally FROM_GC(gc) */
@@ -879,7 +879,9 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
879879
PyGC_Head *next;
880880
int num_freed = 0;
881881

882-
gc_list_init(&wrcb_to_call);
882+
if (allow_callbacks) {
883+
gc_list_init(&wrcb_to_call);
884+
}
883885

884886
/* Clear all weakrefs to the objects in unreachable. If such a weakref
885887
* also has a callback, move it into `wrcb_to_call` if the callback
@@ -935,6 +937,11 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
935937
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
936938
_PyWeakref_ClearRef(wr);
937939
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
940+
941+
if (!allow_callbacks) {
942+
continue;
943+
}
944+
938945
if (wr->wr_callback == NULL) {
939946
/* no callback */
940947
continue;
@@ -987,6 +994,10 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
987994
}
988995
}
989996

997+
if (!allow_callbacks) {
998+
return 0;
999+
}
1000+
9901001
/* Invoke the callbacks we decided to honor. It's safe to invoke them
9911002
* because they can't reference unreachable objects.
9921003
*/
@@ -1737,7 +1748,7 @@ gc_collect_region(PyThreadState *tstate,
17371748
}
17381749

17391750
/* Clear weakrefs and invoke callbacks as necessary. */
1740-
stats->collected += handle_weakrefs(&unreachable, to);
1751+
stats->collected += handle_weakrefs(&unreachable, to, true);
17411752
gc_list_validate_space(to, gcstate->visited_space);
17421753
validate_list(to, collecting_clear_unreachable_clear);
17431754
validate_list(&unreachable, collecting_set_unreachable_clear);
@@ -1751,6 +1762,14 @@ gc_collect_region(PyThreadState *tstate,
17511762
gc_list_init(&final_unreachable);
17521763
handle_resurrected_objects(&unreachable, &final_unreachable, to);
17531764

1765+
/* Clear weakrefs to objects in the unreachable set. No Python-level
1766+
* code must be allowed to access those unreachable objects. During
1767+
* delete_garbage(), finalizers outside the unreachable set might run
1768+
* and create new weakrefs. If those weakrefs were not cleared, they
1769+
* could reveal unreachable objects. Callbacks are not executed.
1770+
*/
1771+
handle_weakrefs(&final_unreachable, NULL, false);
1772+
17541773
/* Call tp_clear on objects in the final_unreachable set. This will cause
17551774
* the reference cycles to be broken. It may also cause some objects
17561775
* in finalizers to be freed.

Python/gc_free_threading.c

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

14941494
// Clear all weakrefs to unreachable objects. Weakrefs with callbacks are
1495-
// enqueued in `wrcb_to_call`, but not invoked yet.
1495+
// optionally enqueued in `wrcb_to_call`, but not invoked yet.
14961496
static void
1497-
clear_weakrefs(struct collection_state *state)
1497+
clear_weakrefs(struct collection_state *state, bool enqueue_callbacks)
14981498
{
14991499
PyObject *op;
15001500
WORKSTACK_FOR_EACH(&state->unreachable, op) {
@@ -1526,6 +1526,10 @@ clear_weakrefs(struct collection_state *state)
15261526
_PyWeakref_ClearRef(wr);
15271527
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
15281528

1529+
if (!enqueue_callbacks) {
1530+
continue;
1531+
}
1532+
15291533
// We do not invoke callbacks for weakrefs that are themselves
15301534
// unreachable. This is partly for hi E6D2 storical reasons: weakrefs
15311535
// predate safe object finalization, and a weakref that is itself
@@ -2211,7 +2215,7 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
22112215
interp->gc.long_lived_total = state->long_lived_total;
22122216

22132217
// Clear weakrefs and enqueue callbacks (but do not call them).
2214-
clear_weakrefs(state);
2218+
clear_weakrefs(state, true);
22152219
_PyEval_StartTheWorld(interp);
22162220

22172221
// Deallocate any object from the refcount merge step
@@ -2222,11 +2226,19 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
22222226
call_weakref_callbacks(state);
22232227
finalize_garbage(state);
22242228

2225-
// Handle any objects that may have resurrected after the finalization.
22262229
_PyEval_StopTheWorld(interp);
2230+
// Handle any objects that may have resurrected after the finalization.
22272231
err = handle_resurrected_objects(state);
22282232
// Clear free lists in all threads
22292233
_PyGC_ClearAllFreeLists(interp);
2234+
if (err == 0) {
2235+
// Clear weakrefs to objects in the unreachable set. No Python-level
2236+
// code must be allowed to access those unreachable objects. During
2237+
// delete_garbage(), finalizers outside the unreachable set might
2238+
// run and create new weakrefs. If those weakrefs were not cleared,
2239+
// they could reveal unreachable objects.
2240+
clear_weakrefs(state, false);
2241+
}
22302242
_PyEval_StartTheWorld(interp);
22312243

22322244
if (err < 0) {

0 commit comments

Comments
 (0)
0