Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -3206,6 +3206,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 @@ -3415,6 +3468,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 @@ -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()
Expand All @@ -1447,9 +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) {
// No generation needs to be collected.
_Py_atomic_store_int(&gcstate->collecting, 0);
return 0;
goto exit;
}
}

Expand Down Expand Up @@ -1643,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:
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