diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index f13bc2178b1e7e..aa35d7c1f14cdb 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -229,6 +229,7 @@ struct _gc_runtime_state { #ifndef Py_GIL_DISABLED struct gc_generation generations[NUM_GENERATIONS]; #else + PyMutex generations_mutex; // Mutex to guard threshold access to the generations below. struct gc_generation young; struct gc_generation old[2]; #endif diff --git a/Lib/test/test_free_threading/test_gc.py b/Lib/test/test_free_threading/test_gc.py index 8b45b6e2150c28..cc1888dae48bc0 100644 --- a/Lib/test/test_free_threading/test_gc.py +++ b/Lib/test/test_free_threading/test_gc.py @@ -94,6 +94,36 @@ def evil(): thread.start() thread.join() + def test_set_threshold(self): + # GH-148613: Setting the GC threshold from another thread could cause a + # race between the `gc_should_collect` and `gc_set_threshold` functions. + NUM_THREADS = 8 + NUM_ITERS = 100_000 + barrier = threading.Barrier(NUM_THREADS) + + class CyclicReference: + def __init__(self): + self.r = self + + def allocator(): + barrier.wait() + for _ in range(NUM_ITERS): + CyclicReference() + + def setter(): + barrier.wait() + for i in range(NUM_ITERS): + gc.set_threshold(100 + (i % 100), 10 + (i % 10), 10 + (i % 10)) + + current_threshold = gc.get_threshold() + try: + threads = [Thread(target=allocator) for _ in range(NUM_THREADS - 1)] + threads.append(Thread(target=setter)) + with threading_helper.start_threads(threads): + pass + finally: + gc.set_threshold(*current_threshold) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-05-24-22-46-49.gh-issue-148613.PLpmyd.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-05-24-22-46-49.gh-issue-148613.PLpmyd.rst new file mode 100644 index 00000000000000..71a701bf3eb355 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-05-24-22-46-49.gh-issue-148613.PLpmyd.rst @@ -0,0 +1,2 @@ +Fix a data race in the free-threaded build between :func:`gc.set_threshold` +and garbage collection scheduling during object allocation. diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 12f93ac0fdea14..dd82893951b9ec 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -167,6 +167,7 @@ gc_set_threshold_impl(PyObject *module, int threshold0, int group_right_1, gcstate->generations[2].threshold = threshold2; } #else + PyMutex_Lock(&gcstate->generations_mutex); gcstate->young.threshold = threshold0; if (group_right_1) { gcstate->old[0].threshold = threshold1; @@ -174,6 +175,7 @@ gc_set_threshold_impl(PyObject *module, int threshold0, int group_right_1, if (group_right_2) { gcstate->old[1].threshold = threshold2; } + PyMutex_Unlock(&gcstate->generations_mutex); #endif Py_RETURN_NONE; } @@ -195,10 +197,15 @@ gc_get_threshold_impl(PyObject *module) gcstate->generations[1].threshold, gcstate->generations[2].threshold); #else + PyMutex_Lock(&gcstate->generations_mutex); + int young_threshold = gcstate->young.threshold; + int old_0_threshold = gcstate->old[0].threshold; + int old_1_threshold = gcstate->old[1].threshold; + PyMutex_Unlock(&gcstate->generations_mutex); return Py_BuildValue("(iii)", - gcstate->young.threshold, - gcstate->old[0].threshold, - gcstate->old[1].threshold); + young_threshold, + old_0_threshold, + old_1_threshold); #endif } diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 4e36189580bbf8..52a22b26781aed 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -1995,13 +1995,22 @@ cleanup_worklist(struct worklist *worklist) static bool gc_should_collect(GCState *gcstate) { + int gc_enabled = _Py_atomic_load_int_relaxed(&gcstate->enabled); + if (!gc_enabled) { + return false; + } + int count = _Py_atomic_load_int_relaxed(&gcstate->young.count); + + PyMutex_Lock(&gcstate->generations_mutex); int threshold = gcstate->young.threshold; - int gc_enabled = _Py_atomic_load_int_relaxed(&gcstate->enabled); - if (count <= threshold || threshold == 0 || !gc_enabled) { + int old_0_threshold = gcstate->old[0].threshold; + PyMutex_Unlock(&gcstate->generations_mutex); + + if (count <= threshold || threshold == 0) { return false; } - if (gcstate->old[0].threshold == 0) { + if (old_0_threshold == 0) { // A few tests rely on immediate scheduling of the GC so we ignore the // extra conditions if generations[1].threshold is set to zero. return true;