Skip to content

Commit

Permalink
pythongh-125859: Fix crash when gc.get_objects is called during GC
Browse files Browse the repository at this point in the history
This fixes a crash when `gc.get_objects()` or `gc.get_referrers()` is
called during a GC in the free threading build. Switch to
`_PyObjectStack` to avoid corrupting the `struct worklist` linked list
maintained by the GC. Also, don't return objects that are frozen
(gc.freeze) or in the process of being collected to more closely match
the behavior of the default build.
  • Loading branch information
colesbury committed Oct 23, 2024
1 parent aaed91c commit aa21b07
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 55 deletions.
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

0 comments on commit aa21b07

Please sign in to comment.