[3.14] Revert "GH-91636: Clear weakrefs created by finalizers. (GH-136401) (#141993)" (#142152)

This commit is contained in:
Hugo van Kemenade 2025-12-01 20:34:37 +02:00 committed by GitHub
parent 4ce27904b5
commit 7642070807
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 14 additions and 54 deletions

View file

@ -262,11 +262,9 @@ class Cyclic(tuple):
# finalizer. # finalizer.
def __del__(self): def __del__(self):
# 5. Create a weakref to `func` now. In previous # 5. Create a weakref to `func` now. If we had created
# versions of Python, this would avoid having it # it earlier, it would have been cleared by the
# cleared by the garbage collector before calling # garbage collector before calling the finalizers.
# the finalizers. Now, weakrefs get cleared after
# calling finalizers.
self[1].ref = weakref.ref(self[0]) self[1].ref = weakref.ref(self[0])
# 6. Drop the global reference to `latefin`. The only # 6. Drop the global reference to `latefin`. The only
@ -295,18 +293,14 @@ def func():
# which will find `cyc` and `func` as garbage. # which will find `cyc` and `func` as garbage.
gc.collect() gc.collect()
# 9. Previously, this would crash because the weakref # 9. Previously, this would crash because `func_qualname`
# created in the finalizer revealed the function after # had been NULL-ed out by func_clear().
# `tp_clear` was called and `func_qualname`
# had been NULL-ed out by func_clear(). Now, we clear
# weakrefs to unreachable objects before calling `tp_clear`
# but after calling finalizers.
print(f"{func=}") print(f"{func=}")
""" """
# We're mostly just checking that this doesn't crash.
rc, stdout, stderr = assert_python_ok("-c", code) rc, stdout, stderr = assert_python_ok("-c", code)
self.assertEqual(rc, 0) self.assertEqual(rc, 0)
# The `func` global is None because the weakref was cleared. self.assertRegex(stdout, rb"""\A\s*func=<function at \S+>\s*\z""")
self.assertRegex(stdout, rb"""\A\s*func=None""")
self.assertFalse(stderr) self.assertFalse(stderr)
@refcount_test @refcount_test

View file

@ -1,3 +0,0 @@
While performing garbage collection, clear weakrefs to unreachable objects
that are created during running of finalizers. If those weakrefs were are
not cleared, they could reveal unreachable objects.

View file

@ -870,7 +870,7 @@ move_legacy_finalizer_reachable(PyGC_Head *finalizers)
* no object in `unreachable` is weakly referenced anymore. * no object in `unreachable` is weakly referenced anymore.
*/ */
static int static int
handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old, bool allow_callbacks) handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
{ {
PyGC_Head *gc; PyGC_Head *gc;
PyObject *op; /* generally FROM_GC(gc) */ PyObject *op; /* generally FROM_GC(gc) */
@ -879,9 +879,7 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old, bool allow_callbacks)
PyGC_Head *next; PyGC_Head *next;
int num_freed = 0; int num_freed = 0;
if (allow_callbacks) { gc_list_init(&wrcb_to_call);
gc_list_init(&wrcb_to_call);
}
/* Clear all weakrefs to the objects in unreachable. If such a weakref /* Clear all weakrefs to the objects in unreachable. If such a weakref
* also has a callback, move it into `wrcb_to_call` if the callback * also has a callback, move it into `wrcb_to_call` if the callback
@ -937,11 +935,6 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old, bool allow_callbacks)
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == op); _PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
_PyWeakref_ClearRef(wr); _PyWeakref_ClearRef(wr);
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None); _PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
if (!allow_callbacks) {
continue;
}
if (wr->wr_callback == NULL) { if (wr->wr_callback == NULL) {
/* no callback */ /* no callback */
continue; continue;
@ -994,10 +987,6 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old, bool allow_callbacks)
} }
} }
if (!allow_callbacks) {
return 0;
}
/* Invoke the callbacks we decided to honor. It's safe to invoke them /* Invoke the callbacks we decided to honor. It's safe to invoke them
* because they can't reference unreachable objects. * because they can't reference unreachable objects.
*/ */
@ -1750,7 +1739,7 @@ gc_collect_region(PyThreadState *tstate,
} }
/* Clear weakrefs and invoke callbacks as necessary. */ /* Clear weakrefs and invoke callbacks as necessary. */
stats->collected += handle_weakrefs(&unreachable, to, true); stats->collected += handle_weakrefs(&unreachable, to);
gc_list_validate_space(to, gcstate->visited_space); gc_list_validate_space(to, gcstate->visited_space);
validate_list(to, collecting_clear_unreachable_clear); validate_list(to, collecting_clear_unreachable_clear);
validate_list(&unreachable, collecting_set_unreachable_clear); validate_list(&unreachable, collecting_set_unreachable_clear);
@ -1764,14 +1753,6 @@ gc_collect_region(PyThreadState *tstate,
gc_list_init(&final_unreachable); gc_list_init(&final_unreachable);
handle_resurrected_objects(&unreachable, &final_unreachable, to); handle_resurrected_objects(&unreachable, &final_unreachable, to);
/* Clear weakrefs to objects in the unreachable set. No Python-level
* code must be allowed to access those unreachable objects. During
* delete_garbage(), finalizers outside the unreachable set might run
* and create new weakrefs. If those weakrefs were not cleared, they
* could reveal unreachable objects. Callbacks are not executed.
*/
handle_weakrefs(&final_unreachable, NULL, false);
/* Call tp_clear on objects in the final_unreachable set. This will cause /* Call tp_clear on objects in the final_unreachable set. This will cause
* the reference cycles to be broken. It may also cause some objects * the reference cycles to be broken. It may also cause some objects
* in finalizers to be freed. * in finalizers to be freed.

View file

@ -1493,9 +1493,9 @@ move_legacy_finalizer_reachable(struct collection_state *state)
} }
// Clear all weakrefs to unreachable objects. Weakrefs with callbacks are // Clear all weakrefs to unreachable objects. Weakrefs with callbacks are
// optionally enqueued in `wrcb_to_call`, but not invoked yet. // enqueued in `wrcb_to_call`, but not invoked yet.
static void static void
clear_weakrefs(struct collection_state *state, bool enqueue_callbacks) clear_weakrefs(struct collection_state *state)
{ {
PyObject *op; PyObject *op;
WORKSTACK_FOR_EACH(&state->unreachable, op) { WORKSTACK_FOR_EACH(&state->unreachable, op) {
@ -1527,10 +1527,6 @@ clear_weakrefs(struct collection_state *state, bool enqueue_callbacks)
_PyWeakref_ClearRef(wr); _PyWeakref_ClearRef(wr);
_PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None); _PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
if (!enqueue_callbacks) {
continue;
}
// We do not invoke callbacks for weakrefs that are themselves // We do not invoke callbacks for weakrefs that are themselves
// unreachable. This is partly for historical reasons: weakrefs // unreachable. This is partly for historical reasons: weakrefs
// predate safe object finalization, and a weakref that is itself // predate safe object finalization, and a weakref that is itself
@ -2216,7 +2212,7 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
interp->gc.long_lived_total = state->long_lived_total; interp->gc.long_lived_total = state->long_lived_total;
// Clear weakrefs and enqueue callbacks (but do not call them). // Clear weakrefs and enqueue callbacks (but do not call them).
clear_weakrefs(state, true); clear_weakrefs(state);
_PyEval_StartTheWorld(interp); _PyEval_StartTheWorld(interp);
// Deallocate any object from the refcount merge step // Deallocate any object from the refcount merge step
@ -2227,19 +2223,11 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
call_weakref_callbacks(state); call_weakref_callbacks(state);
finalize_garbage(state); finalize_garbage(state);
_PyEval_StopTheWorld(interp);
// Handle any objects that may have resurrected after the finalization. // Handle any objects that may have resurrected after the finalization.
_PyEval_StopTheWorld(interp);
err = handle_resurrected_objects(state); err = handle_resurrected_objects(state);
// Clear free lists in all threads // Clear free lists in all threads
_PyGC_ClearAllFreeLists(interp); _PyGC_ClearAllFreeLists(interp);
if (err == 0) {
// Clear weakrefs to objects in the unreachable set. No Python-level
// code must be allowed to access those unreachable objects. During
// delete_garbage(), finalizers outside the unreachable set might
// run and create new weakrefs. If those weakrefs were not cleared,
// they could reveal unreachable objects.
clear_weakrefs(state, false);
}
_PyEval_StartTheWorld(interp); _PyEval_StartTheWorld(interp);
if (err < 0) { if (err < 0) {