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
62 changes: 62 additions & 0 deletions Lib/test/test_free_threading/test_gc_debug_race.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# gh-153014: gc.set_debug() writes gcstate->debug without a lock, racing
# gc.get_debug() and the collector. The flag is an int, so the race stays
# benign, which lets this stress test double as a regression test under a
# free-threading ThreadSanitizer build.

import gc
import threading
import unittest

from test.support import threading_helper


NUM_WRITERS = 2
NUM_READERS = 4
ITERATIONS = 50


def _stress_debug_race(num_writers=NUM_WRITERS, num_readers=NUM_READERS,
iterations=ITERATIONS):
done = threading.Event()

def writer():
try:
for _ in range(iterations):
# DEBUG_SAVEALL avoids the stderr spam DEBUG_STATS would emit.
gc.set_debug(gc.DEBUG_SAVEALL)
gc.set_debug(0)
finally:
done.set()

def reader():
while not done.is_set():
gc.get_debug()

def collector():
while not done.is_set():
a = {}
b = {}
a["b"] = b
b["a"] = a
del a, b
gc.collect()

threading_helper.run_concurrently(
[writer] * num_writers + [reader] * num_readers + [collector]
)


@threading_helper.requires_working_threading()
class TestGCDebugRace(unittest.TestCase):
def setUp(self):
self.addCleanup(gc.garbage.clear)
self.addCleanup(gc.set_debug, gc.get_debug())

def test_set_get_debug_race(self):
_stress_debug_race()
self.assertIsInstance(gc.get_debug(), int)


if __name__ == "__main__":
_stress_debug_race()
print("Done. Run under a free-threading + TSAN build to observe the race.")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a data race on the garbage collector debug flag in free-threading builds:
:func:`gc.set_debug` and :func:`gc.get_debug` now access it atomically.
5 changes: 3 additions & 2 deletions Modules/gcmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "Python.h"
#include "pycore_gc.h"
#include "pycore_object.h" // _PyObject_IS_GC()
#include "pycore_pyatomic_ft_wrappers.h"
#include "pycore_pystate.h" // _PyInterpreterState_GET()

typedef struct _gc_runtime_state GCState;
Expand Down Expand Up @@ -116,7 +117,7 @@ gc_set_debug_impl(PyObject *module, int flags)
/*[clinic end generated code: output=7c8366575486b228 input=5e5ce15e84fbed15]*/
{
GCState *gcstate = get_gc_state();
gcstate->debug = flags;
FT_ATOMIC_STORE_INT_RELAXED(gcstate->debug, flags);
Py_RETURN_NONE;
}

Expand All @@ -131,7 +132,7 @@ gc_get_debug_impl(PyObject *module)
/*[clinic end generated code: output=91242f3506cd1e50 input=91a101e1c3b98366]*/
{
GCState *gcstate = get_gc_state();
return gcstate->debug;
return FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug);
}

/*[clinic input]
Expand Down
19 changes: 10 additions & 9 deletions Python/gc_free_threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "pycore_interp.h" // PyInterpreterState.gc
#include "pycore_interpframe.h" // _PyFrame_GetLocalsArray()
#include "pycore_object_alloc.h" // _PyObject_MallocWithType()
#include "pycore_pyatomic_ft_wrappers.h"
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_tstate.h" // _PyThreadStateImpl
#include "pycore_tuple.h" // _PyTuple_MaybeUntrack()
Expand Down Expand Up @@ -1749,7 +1750,7 @@ delete_garbage(struct collection_state *state)

state->collected++;

if (gcstate->debug & _PyGC_DEBUG_SAVEALL) {
if (FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug) & _PyGC_DEBUG_SAVEALL) {
assert(gcstate->garbage != NULL);
if (PyList_Append(gcstate->garbage, op) < 0) {
_PyErr_Clear(tstate);
Expand Down Expand Up @@ -1780,11 +1781,11 @@ handle_legacy_finalizers(struct collection_state *state)
while ((op = worklist_pop(&state->legacy_finalizers)) != NULL) {
state->uncollectable++;

if (gcstate->debug & _PyGC_DEBUG_UNCOLLECTABLE) {
if (FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug) & _PyGC_DEBUG_UNCOLLECTABLE) {
debug_cycle("uncollectable", op);
}

if ((gcstate->debug & _PyGC_DEBUG_SAVEALL) || has_legacy_finalizer(op)) {
if ((FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug) & _PyGC_DEBUG_SAVEALL) || has_legacy_finalizer(op)) {
if (PyList_Append(gcstate->garbage, op) < 0) {
PyErr_Clear();
}
Expand Down Expand Up @@ -2129,7 +2130,7 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
#endif

// Print debugging information.
if (interp->gc.debug & _PyGC_DEBUG_COLLECTABLE) {
if (FT_ATOMIC_LOAD_INT_RELAXED(interp->gc.debug) & _PyGC_DEBUG_COLLECTABLE) {
PyObject *op;
WORKSTACK_FOR_EACH(&state->unreachable, op) {
debug_cycle("collectable", op);
Expand Down Expand Up @@ -2235,7 +2236,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
invoke_gc_callback(tstate, "start", generation, 0, 0, 0, 0.0);
}

if (gcstate->debug & _PyGC_DEBUG_STATS) {
if (FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug) & _PyGC_DEBUG_STATS) {
PySys_WriteStderr("gc: collecting generation %d...\n", generation);
show_stats_each_generations(gcstate);
}
Expand All @@ -2262,7 +2263,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
(void)PyTime_PerfCounterRaw(&stop);
double duration = PyTime_AsSecondsDouble(stop - start);

if (gcstate->debug & _PyGC_DEBUG_STATS) {
if (FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug) & _PyGC_DEBUG_STATS) {
PySys_WriteStderr(
"gc: done, %zd unreachable, %zd uncollectable, %.4fs elapsed\n",
n+m, n, duration);
Expand Down Expand Up @@ -2586,10 +2587,10 @@ void
_PyGC_DumpShutdownStats(PyInterpreterState *interp)
{
GCState *gcstate = &interp->gc;
if (!(gcstate->debug & _PyGC_DEBUG_SAVEALL)
if (!(FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug) & _PyGC_DEBUG_SAVEALL)
&& gcstate->garbage != NULL && PyList_GET_SIZE(gcstate->garbage) > 0) {
const char *message;
if (gcstate->debug & _PyGC_DEBUG_UNCOLLECTABLE) {
if (FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug) & _PyGC_DEBUG_UNCOLLECTABLE) {
message = "gc: %zd uncollectable objects at shutdown";
}
else {
Expand All @@ -2605,7 +2606,7 @@ _PyGC_DumpShutdownStats(PyInterpreterState *interp)
{
PyErr_FormatUnraisable("Exception ignored in GC shutdown");
}
if (gcstate->debug & _PyGC_DEBUG_UNCOLLECTABLE) {
if (FT_ATOMIC_LOAD_INT_RELAXED(gcstate->debug) & _PyGC_DEBUG_UNCOLLECTABLE) {
PyObject *repr = NULL, *bytes = NULL;
repr = PyObject_Repr(gcstate->garbage);
if (!repr || !(bytes = PyUnicode_EncodeFSDefault(repr))) {
Expand Down
Loading