gh-159437: Skip unnecessary GC finalizer scans

Author: mfalco-oai

Lib/test/test_gc.py

@@ -330,6 +330,7 @@ def f():
         self.assertEqual(gc.collect(), 1)
 
     def test_saveall(self):
+        self.addCleanup(gc.collect)
         # Verify that cyclic garbage like lists show up in gc.garbage if the
         # SAVEALL option is enabled.
 
@@ -352,6 +353,31 @@ def test_saveall(self):
         self.assertEqual(len(gc.garbage), 1)
         obj = gc.garbage.pop()
         self.assertEqual(id(obj), id_L)
+        obj.clear()
+
+        # A no-op generator finalizer must still set the finalized marker.
+        gen = (item for item in ())
+        next(gen, None)
+        self.assertTrue(gc.is_tracked(gen))
+        self.assertIsNone(gen.gi_frame)
+        self.assertFalse(gc.is_finalized(gen))
+        L = [gen]
+        L.append(L)
+        id_L = id(L)
+        del gen
+
+        try:
+            gc.set_debug(gc.DEBUG_SAVEALL)
+            del L
+            gc.collect()
+            saved = next(obj for obj in gc.garbage if id(obj) == id_L)
+            self.assertTrue(gc.is_finalized(saved[0]))
+        finally:
+            gc.set_debug(debug)
+            for obj in gc.garbage:
+                if id(obj) == id_L:
+                    obj.clear()
+            gc.garbage.clear()
 
     def test_del(self):
         # __del__ methods can trigger collection, make this to happen
@@ -658,6 +684,83 @@ def __del__(self):
         lazarus = storage.pop()
         self.assertTrue(gc.is_finalized(lazarus))
 
+    def test_weakref_callback_installs_finalizer(self):
+        """A callback can make a finalizer eligible before the object is cleared."""
+        self.addCleanup(gc.collect)
+        events = []
+
+        class C:
+            """A self-cycle whose weakref callback installs its finalizer."""
+            __slots__ = ("cycle", "__weakref__")
+
+        def finalize(obj):
+            """Record whether finalization sees the intact cycle."""
+            events.append(("finalizer", getattr(obj, "cycle", None) is obj))
+
+        def callback(reference):
+            """Install the finalizer after observing the cleared weakref."""
+            events.append(("callback", reference() is None))
+            C.__del__ = finalize
+
+        obj = C()
+        obj.cycle = obj
+        reference = weakref.ref(obj, callback)
+        gc.collect()
+        del obj
+
+        self.assertEqual(gc.collect(), 1)
+        self.assertEqual(events, [("callback", True), ("finalizer", True)])
+        self.assertIsNone(reference())
+
+    @requires_gil_enabled("tests the generation-list collector")
+    def test_debug_output_resurrects_without_callbacks(self):
+        """Debug output can resurrect garbage without pending callbacks."""
+        was_enabled = gc.isenabled()
+        gc.disable()
+        self.addCleanup(gc.enable if was_enabled else gc.disable)
+        self.addCleanup(gc.collect)
+        resurrected = []
+
+        class C:
+            """An object with a cycle and no finalizer."""
+            __slots__ = ("cycle", "__weakref__")
+
+        class Stderr:
+            """Retain the target when collection reports it as collectable."""
+
+            def write(self, text):
+                """Retain the target on collectable debug output.
+
+                text: Debug text written by the collector.
+                Returns: Number of characters accepted.
+                """
+                if text.startswith("gc: collectable") and not resurrected:
+                    target = reference()
+                    if target is not None:
+                        resurrected.append(target)
+                return len(text)
+
+        writer = Stderr()
+        gc.collect()
+        obj = C()
+        obj.cycle = obj
+        reference = weakref.ref(obj)
+        del obj
+
+        debug = gc.get_debug()
+        try:
+            with unittest.mock.patch.object(sys, "stderr", writer):
+                gc.set_debug(gc.DEBUG_COLLECTABLE)
+                self.assertEqual(gc.collect(), 0)
+            self.assertEqual(len(resurrected), 1)
+            self.assertIs(reference(), resurrected[0])
+            self.assertIs(resurrected[0].cycle, resurrected[0])
+        finally:
+            gc.set_debug(debug)
+            for obj in resurrected:
+                obj.cycle = None
+            resurrected.clear()
+
     def test_bug1055820b(self):
         # Corresponds to temp2b.py in the bug report.
 

Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-23-43-15.gh-issue-159437.LHuMLA.rst

@@ -0,0 +1,3 @@
+With the GIL enabled, garbage collection skips the finalization scan when no
+finalizers or weakref callbacks are pending and debug output for collectable
+objects is disabled.

Python/gc.c

@@ -756,7 +756,7 @@ move_legacy_finalizer_reachable(PyGC_Head *finalizers)
     }
 }
 
-/* Handle weakref callbacks.  Note that it's possible for such weakrefs to be
+/* Prepare weakref callbacks.  Note that it's possible for such weakrefs to be
  * outside the unreachable set -- indeed, those are precisely the weakrefs
  * whose callbacks must be invoked.  See gc_weakref.txt for overview & some
  * details.
@@ -794,17 +794,20 @@ move_legacy_finalizer_reachable(PyGC_Head *finalizers)
  * or missing `tp_traverse` methods.  When that object goes away, the callback
  * for weakref can be executed and that could reveal unreachable objects to
  * Python-level code.  See bpo-38006 as an example bug.
+ *
+ * Arguments:
+ * - `unreachable`: normalized collecting list to inspect.
+ * - `wrcb_to_call`: initialized list receiving weakrefs whose callbacks are safe to invoke.
+ *
+ * Return whether any finalizer is pending or a weakref callback is queued.
+ * This function does not invoke callbacks.
  */
-static int
-handle_weakref_callbacks(PyGC_Head *unreachable, PyGC_Head *old,
-                        bool *needs_resurrection_check)
+Py_NO_INLINE static bool
+prepare_callbacks(PyGC_Head *unreachable, PyGC_Head *wrcb_to_call)
 {
     PyGC_Head *gc;
-    PyGC_Head wrcb_to_call;     /* weakrefs with callbacks to call */
     PyGC_Head *next;
-    int num_freed = 0;
-
-    gc_list_init(&wrcb_to_call);
+    bool has_pending_finalizer = false;
 
     /* Find all weakrefs with callbacks and move into `wrcb_to_call` if the
      * callback needs to be invoked. We make another pass over wrcb_to_call,
@@ -814,9 +817,13 @@ handle_weakref_callbacks(PyGC_Head *unreachable, PyGC_Head *old,
         PyWeakReference **wrlist;
 
         PyObject *op = FROM_GC(gc);
+        PyTypeObject *type = Py_TYPE(op);
+        if (type->tp_finalize != NULL && !has_pending_finalizer) {
+            has_pending_finalizer = !_PyGC_FINALIZED(op);
+        }
         next = GC_NEXT(gc);
 
-        if (! _PyType_SUPPORTS_WEAKREFS(Py_TYPE(op))) {
+        if (! _PyType_SUPPORTS_WEAKREFS(type)) {
             continue;
         }
 
@@ -891,21 +898,35 @@ handle_weakref_callbacks(PyGC_Head *unreachable, PyGC_Head *old,
             PyGC_Head *wrasgc = AS_GC((PyObject *)wr);
             // wrasgc is reachable, but next isn't, so they can't be the same
             _PyObject_ASSERT((PyObject *)wr, wrasgc != next);
-            gc_list_move(wrasgc, &wrcb_to_call);
+            gc_list_move(wrasgc, wrcb_to_call);
         }
     }
 
+    return has_pending_finalizer || !gc_list_is_empty(wrcb_to_call);
+}
+
+/* Invoke queued weakref callbacks and move surviving weakrefs to `old`.
+ *
+ * Arguments:
+ * - `wrcb_to_call`: initialized queue to drain, with a temporary reference to each weakref.
+ * - `old`: generation receiving weakrefs that survive callback invocation.
+ *
+ * Return the number of weakrefs reclaimed while callbacks ran.
+ */
+static int
+invoke_weakref_callbacks(PyGC_Head *wrcb_to_call, PyGC_Head *old)
+{
+    PyGC_Head *gc;
+    int num_freed = 0;
+
     /* Invoke the callbacks we decided to honor.  It's safe to invoke them
      * because they can't reference unreachable objects.
      */
-    if (!*needs_resurrection_check) {
-        *needs_resurrection_check = !gc_list_is_empty(&wrcb_to_call);
-    }
-    while (! gc_list_is_empty(&wrcb_to_call)) {
+    while (! gc_list_is_empty(wrcb_to_call)) {
         PyObject *temp;
         PyObject *callback;
 
-        gc = (PyGC_Head*)wrcb_to_call._gc_next;
+        gc = (PyGC_Head*)wrcb_to_call->_gc_next;
         PyObject *op = FROM_GC(gc);
         _PyObject_ASSERT(op, PyWeakref_Check(op));
         PyWeakReference *wr = (PyWeakReference *)op;
@@ -934,7 +955,7 @@ handle_weakref_callbacks(PyGC_Head *unreachable, PyGC_Head *old,
          * ours).
          */
         Py_DECREF(op);
-        if (wrcb_to_call._gc_next == (uintptr_t)gc) {
+        if (wrcb_to_call->_gc_next == (uintptr_t)gc) {
             /* object is still alive -- move it */
             gc_list_move(gc, old);
         }
@@ -948,7 +969,7 @@ handle_weakref_callbacks(PyGC_Head *unreachable, PyGC_Head *old,
 
 /* Clear all weakrefs to unreachable objects.  When this returns, no object in
  * `unreachable` is weakly referenced anymore.  See the comments above
- * handle_weakref_callbacks() for why these weakrefs need to be cleared.
+ * prepare_callbacks() for why these weakrefs need to be cleared.
  */
 static void
 clear_weakrefs(PyGC_Head *unreachable)
@@ -964,7 +985,7 @@ clear_weakrefs(PyGC_Head *unreachable)
 
         if (PyWeakref_Check(op)) {
             /* A weakref inside the unreachable set is always cleared. See
-             * the comments above handle_weakref_callbacks() for why these
+             * the comments above prepare_callbacks() for why these
              * must be cleared.
              */
             _PyWeakref_ClearRef((PyWeakReference *)op);
@@ -1036,13 +1057,14 @@ handle_legacy_finalizers(PyThreadState *tstate,
 /* Run first-time finalizers (if any) on all the objects in collectable.
  * Note that this may remove some (or even all) of the objects from the
  * list, due to refcounts falling to 0.
+ * Return whether a finalizer may have run Python.
  */
-static void
-finalize_garbage(PyThreadState *tstate, PyGC_Head *collectable,
-                 bool *needs_resurrection_check)
+static bool
+finalize_garbage(PyThreadState *tstate, PyGC_Head *collectable)
 {
     destructor finalize;
     PyGC_Head seen;
+    bool needs_resurrection_check = false;
 
     /* While we're going through the loop, `finalize(op)` may cause op, or
      * other objects, to be reclaimed via refcounts falling to zero.  So
@@ -1062,7 +1084,7 @@ finalize_garbage(PyThreadState *tstate, PyGC_Head *collectable,
             (finalize = Py_TYPE(op)->tp_finalize) != NULL)
         {
             if (!_PyGen_FinalizerIsNoop(op)) {
-                *needs_resurrection_check = true;
+                needs_resurrection_check = true;
             }
             _PyGC_SET_FINALIZED(op);
             Py_INCREF(op);
@@ -1072,6 +1094,7 @@ finalize_garbage(PyThreadState *tstate, PyGC_Head *collectable,
         }
     }
     gc_list_merge(&seen, collectable);
+    return needs_resurrection_check;
 }
 
 /* Break reference cycles by clearing the containers involved.  This is
@@ -1253,6 +1276,43 @@ handle_resurrected_objects(PyGC_Head *unreachable, PyGC_Head* still_unreachable,
 }
 
 
+/* Invoke weakref callbacks and finalizers, then handle possible resurrection.
+ *
+ * Arguments:
+ * - `tstate`: the thread state used when invoking finalizers.
+ * - `collectable`: normalized collecting list whose finalizers may run.
+ * - `still_unreachable`: distinct, uninitialized list head for remaining garbage.
+ * - `old`: generation receiving surviving weakrefs and resurrected objects.
+ * - `wrcb_to_call`: initialized, possibly empty list of callback-bearing weakrefs to drain.
+ * - `collected_count`: count incremented by weakrefs reclaimed while callbacks run.
+ * - `needs_resurrection_check`: whether earlier work may have run Python.
+ *
+ * Return whether resurrection analysis ran. If true, `collectable` must not be
+ * used again and `still_unreachable` contains the remaining garbage. Otherwise,
+ * `collectable` remains selected and `still_unreachable` is untouched.
+ */
+Py_NO_INLINE static bool
+handle_callbacks(PyThreadState *tstate, PyGC_Head *collectable,
+                 PyGC_Head *still_unreachable, PyGC_Head *old,
+                 PyGC_Head *wrcb_to_call, Py_ssize_t *collected_count,
+                 bool needs_resurrection_check)
+{
+    bool weakref_callbacks_invoked = !gc_list_is_empty(wrcb_to_call);
+    *collected_count += invoke_weakref_callbacks(wrcb_to_call, old);
+    needs_resurrection_check |= weakref_callbacks_invoked;
+    validate_list(old, collecting_clear_unreachable_clear);
+    validate_list(collectable, collecting_set_unreachable_clear);
+
+    // Weakref callbacks can make finalizers newly eligible after the initial
+    // scan, so we must recheck even when we previously found no pending finalizers.
+    needs_resurrection_check |= finalize_garbage(tstate, collectable);
+    if (needs_resurrection_check) {
+        handle_resurrected_objects(collectable, still_unreachable, old);
+    }
+    return needs_resurrection_check;
+}
+
+
 /* Invoke progress callbacks to notify clients that garbage collection
  * is starting or stopping
  */
@@ -1563,43 +1623,36 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
         }
     }
 
-    /* Clear weakrefs and invoke callbacks as necessary. */
-    stats.collected += handle_weakref_callbacks(&unreachable, old,
-                                               &needs_resurrection_check);
-    validate_list(old, collecting_clear_unreachable_clear);
-    validate_list(&unreachable, collecting_set_unreachable_clear);
-
-    /* Call tp_finalize on objects which have one. */
-    finalize_garbage(tstate, &unreachable, &needs_resurrection_check);
+    PyGC_Head wrcb_to_call;
+    gc_list_init(&wrcb_to_call);
+    bool has_pending_callbacks = prepare_callbacks(&unreachable, &wrcb_to_call);
+    has_pending_callbacks |= needs_resurrection_check;
 
-    /* Handle any objects that may have resurrected after the call
-     * to 'finalize_garbage' and continue the collection with the
-     * objects that are still unreachable */
     PyGC_Head final_unreachable;
-    if (needs_resurrection_check) {
-        handle_resurrected_objects(&unreachable, &final_unreachable, old);
-    }
-    else {
-        /* No Python code ran during finalization, so the original
-         * unreachable set is still valid. */
-        gc_list_init(&final_unreachable);
-        gc_list_merge(&unreachable, &final_unreachable);
+    PyGC_Head *collectable = &unreachable;
+    if (has_pending_callbacks) {
+        bool resurrection_checked = handle_callbacks(tstate, collectable, &final_unreachable, old, &wrcb_to_call, &stats.collected, needs_resurrection_check);
+        if (resurrection_checked) {
+            collectable = &final_unreachable;
+        }
     }
+    validate_list(old, collecting_clear_unreachable_clear);
+    validate_list(collectable, collecting_set_unreachable_clear);
 
     /* 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.
      */
-    clear_weakrefs(&final_unreachable);
+    clear_weakrefs(collectable);
 
-    /* Call tp_clear on objects in the final_unreachable set.  This will cause
+    /* Call tp_clear on objects in the collectable set.  This will cause
     * the reference cycles to be broken.  It may also cause some objects
     * in finalizers to be freed.
     */
-    stats.collected += gc_list_size(&final_unreachable);
-    delete_garbage(tstate, gcstate, &final_unreachable, old);
+    stats.collected += gc_list_size(collectable);
+    delete_garbage(tstate, gcstate, collectable, old);
 
     /* Collect statistics on uncollectable objects found and print
      * debugging information. */