Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-125859: Fix crash when gc.get_objects is called during GC #125882

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
23 changes: 23 additions & 0 deletions Lib/test/test_gc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,29 @@ def test_get_referents_on_capsule(self):
self.assertEqual(len(gc.get_referents(untracked_capsule)), 0)
gc.get_referents(tracked_capsule)

@cpython_only
def test_get_objects_during_gc(self):
# gh-125859: Calling gc.get_objects() or gc.get_referrers() during a
# collection should not crash.
test = self
collected = False

class GetObjectsOnDel:
def __del__(self):
nonlocal collected
collected = True
objs = gc.get_objects()
# NB: can't use "in" here because some objects override __eq__
for obj in objs:
test.assertTrue(obj is not self)
test.assertEqual(gc.get_referrers(self), [])

obj = GetObjectsOnDel()
obj.cycle = obj
del obj

gc.collect()
self.assertTrue(collected)


class IncrementalGCTests(unittest.TestCase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a crash in the free threading build when :func:`gc.get_objects` or
:func:`gc.get_referrers` is called during an in-progress garbage collection.
107 changes: 52 additions & 55 deletions Python/gc_free_threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -1404,7 +1404,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
struct get_referrers_args {
struct visitor_args base;
PyObject *objs;
struct worklist results;
_PyObjectStack results;
};

static int
Expand All @@ -1428,11 +1428,17 @@ visit_get_referrers(const mi_heap_t *heap, const mi_heap_area_t *area,
if (op == NULL) {
return true;
}
if (op->ob_gc_bits & (_PyGC_BITS_UNREACHABLE | _PyGC_BITS_FROZEN)) {
// Exclude unreachable objects (in-progress GC) and frozen
// objects from gc.get_objects() to match the default build.
return true;
}

struct get_referrers_args *arg = (struct get_referrers_args *)args;
if (Py_TYPE(op)->tp_traverse(op, referrersvisit, arg->objs)) {
op->ob_tid = 0; // we will restore the refcount later
worklist_push(&arg->results, op);
if (_PyObjectStack_Push(&arg->results, op) < 0) {
return false;
}
}

return true;
Expand All @@ -1446,43 +1452,36 @@ _PyGC_GetReferrers(PyInterpreterState *interp, PyObject *objs)
return NULL;
}

// NOTE: We can't append to the PyListObject during gc_visit_heaps()
// because PyList_Append() may reclaim an abandoned mimalloc segments
// while we are traversing them.
struct get_referrers_args args = { .objs = objs };
_PyEval_StopTheWorld(interp);
int err = gc_visit_heaps(interp, &visit_get_referrers, &args.base);
_PyEval_StartTheWorld(interp);

// Append all objects to a worklist. This abuses ob_tid. We will restore
// it later. NOTE: We can't append to the PyListObject during
// gc_visit_heaps() because PyList_Append() may reclaim an abandoned
// mimalloc segments while we are traversing them.
struct get_referrers_args args = { .objs = objs };
gc_visit_heaps(interp, &visit_get_referrers, &args.base);
if (err < 0) {
PyErr_NoMemory();
goto error;
}

bool error = false;
PyObject *op;
while ((op = worklist_pop(&args.results)) != NULL) {
gc_restore_tid(op);
while ((op = _PyObjectStack_Pop(&args.results)) != NULL) {
if (op != objs && PyList_Append(result, op) < 0) {
error = true;
break;
goto error;
}
}

// In case of error, clear the remaining worklist
while ((op = worklist_pop(&args.results)) != NULL) {
gc_restore_tid(op);
}

_PyEval_StartTheWorld(interp);

if (error) {
Py_DECREF(result);
return NULL;
}

return result;

error:
Py_DECREF(result);
_PyObjectStack_Clear(&args.results);
return NULL;
}

struct get_objects_args {
struct visitor_args base;
struct worklist objects;
_PyObjectStack objects;
};

static bool
Expand All @@ -1493,11 +1492,16 @@ visit_get_objects(const mi_heap_t *heap, const mi_heap_area_t *area,
if (op == NULL) {
return true;
}
if (op->ob_gc_bits & (_PyGC_BITS_UNREACHABLE | _PyGC_BITS_FROZEN)) {
// Exclude unreachable objects (in-progress GC) and frozen
// objects from gc.get_objects() to match the default build.
return true;
}

struct get_objects_args *arg = (struct get_objects_args *)args;
op->ob_tid = 0; // we will restore the refcount later
worklist_push(&arg->objects, op);

if (_PyObjectStack_Push(&arg->objects, op) < 0) {
return false;
}
return true;
}

Expand All @@ -1509,38 +1513,31 @@ _PyGC_GetObjects(PyInterpreterState *interp, int generation)
return NULL;
}

// NOTE: We can't append to the PyListObject during gc_visit_heaps()
// because PyList_Append() may reclaim an abandoned mimalloc segments
// while we are traversing them.
struct get_objects_args args = { 0 };
_PyEval_StopTheWorld(interp);
int err = gc_visit_heaps(interp, &visit_get_objects, &args.base);
_PyEval_StartTheWorld(interp);

// Append all objects to a worklist. This abuses ob_tid. We will restore
// it later. NOTE: We can't append to the list during gc_visit_heaps()
// because PyList_Append() may reclaim an abandoned mimalloc segment
// while we are traversing it.
struct get_objects_args args = { 0 };
gc_visit_heaps(interp, &visit_get_objects, &args.base);
if (err < 0) {
PyErr_NoMemory();
goto error;
}

bool error = false;
PyObject *op;
while ((op = worklist_pop(&args.objects)) != NULL) {
gc_restore_tid(op);
while ((op = _PyObjectStack_Pop(&args.objects)) != NULL) {
if (op != result && PyList_Append(result, op) < 0) {
error = true;
break;
goto error;
}
}

// In case of error, clear the remaining worklist
while ((op = worklist_pop(&args.objects)) != NULL) {
gc_restore_tid(op);
}

_PyEval_StartTheWorld(interp);

if (error) {
Py_DECREF(result);
return NULL;
}

return result;

error:
Py_DECREF(result);
_PyObjectStack_Clear(&args.objects);
return NULL;
}

static bool
Expand Down
Loading