diff --git a/Include/internal/pycore_object_stack.h b/Include/internal/pycore_object_stack.h index c607ea8bc52545..39e69b7cde52a1 100644 --- a/Include/internal/pycore_object_stack.h +++ b/Include/internal/pycore_object_stack.h @@ -71,6 +71,16 @@ _PyObjectStack_Pop(_PyObjectStack *stack) return obj; } +static inline Py_ssize_t +_PyObjectStack_Size(_PyObjectStack *stack) +{ + Py_ssize_t size = 0; + for (_PyObjectStackChunk *buf = stack->head; buf != NULL; buf = buf->prev) { + size += buf->n; + } + return size; +} + // Merge src into dst, leaving src empty extern void _PyObjectStack_Merge(_PyObjectStack *dst, _PyObjectStack *src); diff --git a/Lib/test/test_free_threading/test_gc.py b/Lib/test/test_free_threading/test_gc.py new file mode 100644 index 00000000000000..401067fe9c612c --- /dev/null +++ b/Lib/test/test_free_threading/test_gc.py @@ -0,0 +1,61 @@ +import unittest + +import threading +from threading import Thread +from unittest import TestCase +import gc + +from test.support import threading_helper + + +class MyObj: + pass + + +@threading_helper.requires_working_threading() +class TestGC(TestCase): + def test_get_objects(self): + event = threading.Event() + + def gc_thread(): + for i in range(100): + o = gc.get_objects() + event.set() + + def mutator_thread(): + while not event.is_set(): + o1 = MyObj() + o2 = MyObj() + o3 = MyObj() + o4 = MyObj() + + gcs = [Thread(target=gc_thread)] + mutators = [Thread(target=mutator_thread) for _ in range(4)] + with threading_helper.start_threads(gcs + mutators): + pass + + def test_get_referrers(self): + event = threading.Event() + + obj = MyObj() + + def gc_thread(): + for i in range(100): + o = gc.get_referrers(obj) + event.set() + + def mutator_thread(): + while not event.is_set(): + d1 = { "key": obj } + d2 = { "key": obj } + d3 = { "key": obj } + d4 = { "key": obj } + + gcs = [Thread(target=gc_thread) for _ in range(2)] + mutators = [Thread(target=mutator_thread) for _ in range(4)] + with threading_helper.start_threads(gcs + mutators): + pass + + +if __name__ == "__main__": + unittest.main() diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index aa8912d721ba70..1969ed608ea524 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -1401,6 +1401,28 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) return n + m; } +static PyObject * +list_from_object_stack(_PyObjectStack *stack) +{ + PyObject *list = PyList_New(_PyObjectStack_Size(stack)); + if (list == NULL) { + PyObject *op; + while ((op = _PyObjectStack_Pop(stack)) != NULL) { + Py_DECREF(op); + } + return NULL; + } + + PyObject *op; + Py_ssize_t idx = 0; + while ((op = _PyObjectStack_Pop(stack)) != NULL) { + assert(idx < PyList_GET_SIZE(list)); + PyList_SET_ITEM(list, idx++, op); + } + assert(idx == PyList_GET_SIZE(list)); + return list; +} + struct get_referrers_args { struct visitor_args base; PyObject *objs; @@ -1435,8 +1457,12 @@ visit_get_referrers(const mi_heap_t *heap, const mi_heap_area_t *area, } struct get_referrers_args *arg = (struct get_referrers_args *)args; + if (op == arg->objs) { + // Don't include the tuple itself in the referrers list. + return true; + } if (Py_TYPE(op)->tp_traverse(op, referrersvisit, arg->objs)) { - if (_PyObjectStack_Push(&arg->results, op) < 0) { + if (_PyObjectStack_Push(&arg->results, Py_NewRef(op)) < 0) { return false; } } @@ -1447,11 +1473,6 @@ visit_get_referrers(const mi_heap_t *heap, const mi_heap_area_t *area, PyObject * _PyGC_GetReferrers(PyInterpreterState *interp, PyObject *objs) { - PyObject *result = PyList_New(0); - if (!result) { - 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. @@ -1460,23 +1481,12 @@ _PyGC_GetReferrers(PyInterpreterState *interp, PyObject *objs) int err = gc_visit_heaps(interp, &visit_get_referrers, &args.base); _PyEval_StartTheWorld(interp); + PyObject *list = list_from_object_stack(&args.results); if (err < 0) { PyErr_NoMemory(); - goto error; + Py_CLEAR(list); } - - PyObject *op; - while ((op = _PyObjectStack_Pop(&args.results)) != NULL) { - if (op != objs && PyList_Append(result, op) < 0) { - goto error; - } - } - return result; - -error: - Py_DECREF(result); - _PyObjectStack_Clear(&args.results); - return NULL; + return list; } struct get_objects_args { @@ -1499,7 +1509,7 @@ visit_get_objects(const mi_heap_t *heap, const mi_heap_area_t *area, } struct get_objects_args *arg = (struct get_objects_args *)args; - if (_PyObjectStack_Push(&arg->objects, op) < 0) { + if (_PyObjectStack_Push(&arg->objects, Py_NewRef(op)) < 0) { return false; } return true; @@ -1508,11 +1518,6 @@ visit_get_objects(const mi_heap_t *heap, const mi_heap_area_t *area, PyObject * _PyGC_GetObjects(PyInterpreterState *interp, int generation) { - PyObject *result = PyList_New(0); - if (!result) { - 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. @@ -1521,23 +1526,12 @@ _PyGC_GetObjects(PyInterpreterState *interp, int generation) int err = gc_visit_heaps(interp, &visit_get_objects, &args.base); _PyEval_StartTheWorld(interp); + PyObject *list = list_from_object_stack(&args.objects); if (err < 0) { PyErr_NoMemory(); - goto error; + Py_CLEAR(list); } - - PyObject *op; - while ((op = _PyObjectStack_Pop(&args.objects)) != NULL) { - if (op != result && PyList_Append(result, op) < 0) { - goto error; - } - } - return result; - -error: - Py_DECREF(result); - _PyObjectStack_Clear(&args.objects); - return NULL; + return list; } static bool