From 9d8fa7e5b43adb3e864921e141a173d283ee5460 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Tue, 8 Sep 2026 17:44:03 +0500 Subject: [PATCH 01/10] Test that GC frame is not cleared --- Lib/test/test_gc.py | 18 ++++++++++++++++++ Modules/_testinternalcapi.c | 14 ++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 10f2a5dfb505e3..36168efafc4ffe 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1289,6 +1289,24 @@ def test_heap_size(self): del l self.assertEqual(count, _testinternalcapi.get_tracked_heap_size()) + @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") + def test_clear_frame_on_early_return(self): + # __del__ methods can trigger collection, make this to happen + thresholds = gc.get_threshold() + gc.enable() + gc.set_threshold(1) + + class A: + def __del__(self): + dir(self) + + x = [A() for _ in range(10)] + del x + self.assertTrue(_testinternalcapi.is_gc_frame_clear()) + + gc.disable() + gc.set_threshold(*thresholds) + class GCCallbackTests(unittest.TestCase): def setUp(self): diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 38e56ae7042098..6de03fba43ba29 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -3206,6 +3206,19 @@ test_thread_state_ensure_from_view_interp_switch(PyObject *self, PyObject *unuse Py_RETURN_NONE; } +static PyObject * +is_gc_frame_clear(PyObject *self, PyObject *unused) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + assert(interp != NULL); + + if (!interp->gc.frame) { + Py_RETURN_TRUE; + } + + Py_RETURN_FALSE; +} + /* Self interrupting context manager */ typedef struct { @@ -3393,6 +3406,7 @@ static PyMethodDef module_functions[] = { {"test_interp_guard_countdown", test_interp_guard_countdown, METH_NOARGS}, {"test_interp_view_countdown", test_interp_view_countdown, METH_NOARGS}, {"test_thread_state_ensure_from_view_interp_switch", test_thread_state_ensure_from_view_interp_switch, METH_NOARGS}, + {"is_gc_frame_clear", is_gc_frame_clear, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From a4b6f3da67e75cc1ae0ac66c7d2649318f5e2178 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Tue, 8 Sep 2026 17:44:16 +0500 Subject: [PATCH 02/10] Clear GC frame on early return --- Python/gc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/gc.c b/Python/gc.c index 201c621bcc3cb9..8b1bdeb56556a2 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -1447,6 +1447,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) // objects from that generation and all generations younger than it. generation = gc_select_generation(gcstate); if (generation < 0) { + gcstate->frame = NULL; // No generation needs to be collected. _Py_atomic_store_int(&gcstate->collecting, 0); return 0; From f15dc69346c693e42eda1449d0895c108cd03492 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Tue, 8 Sep 2026 17:54:44 +0500 Subject: [PATCH 03/10] Add news entry --- .../2026-09-08-17-54-34.gh-issue-156425.-Rn7Tw.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-09-08-17-54-34.gh-issue-156425.-Rn7Tw.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-09-08-17-54-34.gh-issue-156425.-Rn7Tw.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-08-17-54-34.gh-issue-156425.-Rn7Tw.rst new file mode 100644 index 00000000000000..bee2f80335df95 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-08-17-54-34.gh-issue-156425.-Rn7Tw.rst @@ -0,0 +1,3 @@ +Fix a bug in the garbage collector where frames could be kept alive longer +than necessary, potentially distorting profiler statistics. Patch by Sergey +Miryanov. From 1740ca1c8b462940ccd5ccc401270cdc10941773 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Thu, 24 Sep 2026 02:15:52 +0500 Subject: [PATCH 04/10] New repro --- Lib/test/test_gc.py | 17 +++---------- Modules/_testinternalcapi.c | 50 +++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 6c46a56b9cd5be..226ce8fefdb7eb 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1291,21 +1291,12 @@ def test_heap_size(self): @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") def test_clear_frame_on_early_return(self): - # __del__ methods can trigger collection, make this to happen thresholds = gc.get_threshold() gc.enable() - gc.set_threshold(1) - - class A: - def __del__(self): - dir(self) - - x = [A() for _ in range(10)] - del x - self.assertTrue(_testinternalcapi.is_gc_frame_clear()) - - gc.disable() - gc.set_threshold(*thresholds) + try: + self.assertTrue(_testinternalcapi.test_gc_frame_cleared(thresholds[0])) + finally: + gc.disable() class GCCallbackTests(unittest.TestCase): diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index a390fb3ee00593..c078071bcab411 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -3207,11 +3207,57 @@ test_thread_state_ensure_from_view_interp_switch(PyObject *self, PyObject *unuse } static PyObject * -is_gc_frame_clear(PyObject *self, PyObject *unused) +test_gc_frame_cleared(PyObject *self, PyObject *arg) { PyInterpreterState *interp = _PyInterpreterState_GET(); + PyThreadState *tstate = PyThreadState_GET(); assert(interp != NULL); + assert(tstate != NULL); + + int32_t n = 0; + if (0 > PyLong_AsInt32(arg, &n)) { + return NULL; + } + + PyObject *a = PyLong_GetInfo(); + if (a == NULL) { + return NULL; + } + PyObject *list = PyList_New(0); + if (list == NULL) { + Py_DECREF(a); + return NULL; + } + + // We create n+1 objects, which should schedule a GC run. + // However, since we are inside a C call, garbage collection will + // not run immediately. + for (int i = 0; i < n; i++) { + PyObject *b = PyLong_GetInfo(); + if (b == NULL) { + Py_DECREF(a); + Py_DECREF(list); + return NULL; + } + if (0 > PyList_Append(list, b)) { + Py_DECREF(a); + Py_DECREF(list); + Py_DECREF(b); + return NULL; + } + Py_DECREF(b); + } + + // We then clear n objects, which decrements the young generation counter. + assert(0 == PyList_Clear(list)); + Py_DECREF(list); + + // We trigger pending garbage collection. + // However, the GC start conditions are not satisfied (young.threshold > young.count), + // so gc_select_generation() cannot select a generation and returns early. + assert(0 == _Py_HandlePending(tstate)); + Py_DECREF(a); if (!interp->gc.frame) { Py_RETURN_TRUE; } @@ -3428,7 +3474,7 @@ static PyMethodDef module_functions[] = { {"test_interp_guard_countdown", test_interp_guard_countdown, METH_NOARGS}, {"test_interp_view_countdown", test_interp_view_countdown, METH_NOARGS}, {"test_thread_state_ensure_from_view_interp_switch", test_thread_state_ensure_from_view_interp_switch, METH_NOARGS}, - {"is_gc_frame_clear", is_gc_frame_clear, METH_NOARGS}, + {"test_gc_frame_cleared", test_gc_frame_cleared, METH_O}, {"unicodewriter_overflow", unicodewriter_overflow, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From a1903b9143c79bd29e4fe35151f3ca0ebd02c24c Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Thu, 24 Sep 2026 02:17:13 +0500 Subject: [PATCH 05/10] Call gc.collect before test --- Lib/test/test_gc.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 226ce8fefdb7eb..caccc732e5834a 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1294,6 +1294,7 @@ def test_clear_frame_on_early_return(self): thresholds = gc.get_threshold() gc.enable() try: + gc.collect(2) self.assertTrue(_testinternalcapi.test_gc_frame_cleared(thresholds[0])) finally: gc.disable() From 64b51961775ad8890380ddf0f4803ae68a54b2ba Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Thu, 24 Sep 2026 02:21:19 +0500 Subject: [PATCH 06/10] Use goto to exit --- Python/gc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Python/gc.c b/Python/gc.c index 8b1bdeb56556a2..774538fced0d22 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -1429,6 +1429,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) PyGC_Head finalizers; /* objects with, & reachable from, __del__ */ PyGC_Head *gc; GCState *gcstate = &tstate->interp->gc; + Py_ssize_t result = 0; // gc_collect_main() must not be called before _PyGC_Init // or after _PyGC_Fini() @@ -1447,10 +1448,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) // objects from that generation and all generations younger than it. generation = gc_select_generation(gcstate); if (generation < 0) { - gcstate->frame = NULL; - // No generation needs to be collected. - _Py_atomic_store_int(&gcstate->collecting, 0); - return 0; + goto exit; } } @@ -1644,10 +1642,13 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) invoke_gc_callback(tstate, "stop", generation, &stats); } + result = stats.uncollectable + stats.collected; + +exit: assert(!_PyErr_Occurred(tstate)); gcstate->frame = NULL; _Py_atomic_store_int(&gcstate->collecting, 0); - return stats.uncollectable + stats.collected; + return result; } static int From 44ee3c335aeca65a98f02be3780d814c06fd0892 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Thu, 24 Sep 2026 10:35:37 +0500 Subject: [PATCH 07/10] Rename back to is_gc_frame_cleared --- Lib/test/test_gc.py | 2 +- Modules/_testinternalcapi.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index caccc732e5834a..17187b40befad6 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1295,7 +1295,7 @@ def test_clear_frame_on_early_return(self): gc.enable() try: gc.collect(2) - self.assertTrue(_testinternalcapi.test_gc_frame_cleared(thresholds[0])) + self.assertTrue(_testinternalcapi.is_gc_frame_cleared(thresholds[0])) finally: gc.disable() diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index c078071bcab411..e79d6ceabfa794 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -3207,7 +3207,7 @@ test_thread_state_ensure_from_view_interp_switch(PyObject *self, PyObject *unuse } static PyObject * -test_gc_frame_cleared(PyObject *self, PyObject *arg) +is_gc_frame_cleared(PyObject *self, PyObject *arg) { PyInterpreterState *interp = _PyInterpreterState_GET(); PyThreadState *tstate = PyThreadState_GET(); @@ -3474,7 +3474,7 @@ static PyMethodDef module_functions[] = { {"test_interp_guard_countdown", test_interp_guard_countdown, METH_NOARGS}, {"test_interp_view_countdown", test_interp_view_countdown, METH_NOARGS}, {"test_thread_state_ensure_from_view_interp_switch", test_thread_state_ensure_from_view_interp_switch, METH_NOARGS}, - {"test_gc_frame_cleared", test_gc_frame_cleared, METH_O}, + {"is_gc_frame_cleared", is_gc_frame_cleared, METH_O}, {"unicodewriter_overflow", unicodewriter_overflow, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From 5aba59dff31b37984f06e27bcbc175d4972bffb8 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Thu, 24 Sep 2026 22:42:44 +0500 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: Victor Stinner --- Lib/test/test_gc.py | 2 ++ Modules/_testinternalcapi.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 17187b40befad6..d3515e7a39597b 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1291,6 +1291,8 @@ def test_heap_size(self): @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") def test_clear_frame_on_early_return(self): + # gh-156425: Make sure that a garbage collection always clears + # PyInterpreterState.gc.frame when it's done. thresholds = gc.get_threshold() gc.enable() try: diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index e79d6ceabfa794..2c2641e3fb131a 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -3206,6 +3206,8 @@ test_thread_state_ensure_from_view_interp_switch(PyObject *self, PyObject *unuse Py_RETURN_NONE; } +# gh-156425: Make sure that a garbage collection always clears +# PyInterpreterState.gc.frame when it's done. static PyObject * is_gc_frame_cleared(PyObject *self, PyObject *arg) { From 6b9dc5e90d557aaf64a8db45d7653d0223086014 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Thu, 24 Sep 2026 22:46:00 +0500 Subject: [PATCH 09/10] Address review --- Modules/_testinternalcapi.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 2c2641e3fb131a..3402a8b47a793e 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -3206,8 +3206,8 @@ test_thread_state_ensure_from_view_interp_switch(PyObject *self, PyObject *unuse Py_RETURN_NONE; } -# gh-156425: Make sure that a garbage collection always clears -# PyInterpreterState.gc.frame when it's done. +// gh-156425: Make sure that a garbage collection always clears +// PyInterpreterState.gc.frame when it's done. static PyObject * is_gc_frame_cleared(PyObject *self, PyObject *arg) { @@ -3221,28 +3221,21 @@ is_gc_frame_cleared(PyObject *self, PyObject *arg) return NULL; } - PyObject *a = PyLong_GetInfo(); - if (a == NULL) { - return NULL; - } PyObject *list = PyList_New(0); if (list == NULL) { - Py_DECREF(a); return NULL; } - // We create n+1 objects, which should schedule a GC run. + // We create n objects, which should schedule a GC run. // However, since we are inside a C call, garbage collection will // not run immediately. - for (int i = 0; i < n; i++) { + for (int32_t i = 0; i < n; i++) { PyObject *b = PyLong_GetInfo(); if (b == NULL) { - Py_DECREF(a); Py_DECREF(list); return NULL; } if (0 > PyList_Append(list, b)) { - Py_DECREF(a); Py_DECREF(list); Py_DECREF(b); return NULL; @@ -3250,7 +3243,7 @@ is_gc_frame_cleared(PyObject *self, PyObject *arg) Py_DECREF(b); } - // We then clear n objects, which decrements the young generation counter. + // Then we clear n objects, which decrements the young generation counter. assert(0 == PyList_Clear(list)); Py_DECREF(list); @@ -3259,7 +3252,6 @@ is_gc_frame_cleared(PyObject *self, PyObject *arg) // so gc_select_generation() cannot select a generation and returns early. assert(0 == _Py_HandlePending(tstate)); - Py_DECREF(a); if (!interp->gc.frame) { Py_RETURN_TRUE; } From c26e4969c631feae1288cc945b792823f90f24a3 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Thu, 24 Sep 2026 22:48:24 +0500 Subject: [PATCH 10/10] Lint --- Lib/test/test_gc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index d3515e7a39597b..45311db9a4bdbc 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1291,7 +1291,7 @@ def test_heap_size(self): @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") def test_clear_frame_on_early_return(self): - # gh-156425: Make sure that a garbage collection always clears + # gh-156425: Make sure that a garbage collection always clears # PyInterpreterState.gc.frame when it's done. thresholds = gc.get_threshold() gc.enable()