Skip to content
12 changes: 12 additions & 0 deletions Lib/test/test_gc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,18 @@ 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):
# gh-156425: Make sure that a garbage collection always clears
# PyInterpreterState.gc.frame when it's done.
thresholds = gc.get_threshold()
Comment thread
sergey-miryanov marked this conversation as resolved.
gc.enable()
try:
gc.collect(2)
self.assertTrue(_testinternalcapi.is_gc_frame_cleared(thresholds[0]))
finally:
gc.disable()


class GCCallbackTests(unittest.TestCase):
def setUp(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
54 changes: 54 additions & 0 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -3256,6 +3256,59 @@ 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)
Comment thread
sergey-miryanov marked this conversation as resolved.
{
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 *list = PyList_New(0);
if (list == NULL) {
return NULL;
}

// 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 (int32_t i = 0; i < n; i++) {
PyObject *b = PyLong_GetInfo();
if (b == NULL) {
Py_DECREF(list);
return NULL;
}
if (0 > PyList_Append(list, b)) {
Py_DECREF(list);
Py_DECREF(b);
return NULL;
}
Py_DECREF(b);
}

// Then we 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));

if (!interp->gc.frame) {
Py_RETURN_TRUE;
}

Py_RETURN_FALSE;
}

static PyObject *
unicodewriter_overflow(PyObject *self, PyObject *unused)
{
Expand Down Expand Up @@ -3466,6 +3519,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_cleared", is_gc_frame_cleared, METH_O},
{"unicodewriter_overflow", unicodewriter_overflow, METH_NOARGS},
{NULL, NULL} /* sentinel */
};
Expand Down
10 changes: 6 additions & 4 deletions Python/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1437,6 +1437,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()
Expand All @@ -1455,9 +1456,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) {
// No generation needs to be collected.
_Py_atomic_store_int(&gcstate->collecting, 0);
return 0;
goto exit;
}
}

Expand Down Expand Up @@ -1664,10 +1663,13 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
invoke_gc_callback(tstate, "stop", generation, &stats);
}

result = stats.uncollectable + stats.collected;

exit:
Comment thread
sergey-miryanov marked this conversation as resolved.
assert(!_PyErr_Occurred(tstate));
gcstate->frame = NULL;
_Py_atomic_store_int(&gcstate->collecting, 0);
return stats.uncollectable + stats.collected;
return result;
}

static int
Expand Down
Loading