Skip to content

Commit 8b1dbb1

Browse files
zangjiuchengclaude
andauthored
gh-151646: Fix data race between gc.get_stats() and GC in free-threading builds (gh-151766)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 548c731 commit 8b1dbb1

5 files changed

Lines changed: 117 additions & 1 deletion

File tree

Include/internal/pycore_interp_structs.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,12 @@ struct _gc_runtime_state {
235235
/* a permanent generation which won't be collected */
236236
struct gc_generation permanent_generation;
237237
struct gc_stats *generation_stats;
238+
#ifdef Py_GIL_DISABLED
239+
/* Protects generation_stats between gc_get_stats_impl() (reader) and
240+
gc_collect_main() (writer); both resolve the stats slot (buffer index)
241+
under this lock so they agree on which slot to touch (gh-151646). */
242+
PyMutex stats_mutex;
243+
#endif
238244
/* true if we are currently running the collector */
239245
int collecting;
240246
// The frame that started the current collection. It might be NULL even when
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# Reproduction for the gc.get_stats() data race under free-threading.
2+
#
3+
# CPython issue gh-151646: in free-threading builds (``--disable-gil``) the
4+
# concurrent collector and ``gc.get_stats()`` touch the same per-generation
5+
# statistics struct (``gcstate->generation_stats``) without any synchronisation:
6+
#
7+
# * READER -- ``gc_get_stats_impl()`` in ``Modules/gcmodule.c`` copies the
8+
# ``struct gc_generation_stats`` for each generation (``collections``,
9+
# ``collected``, ``uncollectable``, ``candidates``, ``duration``) with a
10+
# plain struct assignment and no lock.
11+
#
12+
# * WRITER -- at the end of every collection cycle ``gc_collect_main()`` in
13+
# ``Python/gc_free_threading.c`` mutates that very struct in place
14+
# (``stats->collections++``, ``stats->collected += m``, ...), again with no
15+
# lock.
16+
#
17+
# When one thread is collecting while several others read the stats, the
18+
# unsynchronised read/write to the same memory is a data race. The values are
19+
# only statistics, so the race is benign at the Python level (no crash), which
20+
# is exactly what lets this script double as a regression test: once the fix
21+
# adds proper synchronisation, ThreadSanitizer should stay quiet while the
22+
# script keeps exiting cleanly.
23+
#
24+
# Running this script under a free-threading CPython build compiled with
25+
# ThreadSanitizer (``./configure --disable-gil --with-thread-sanitizer``) is
26+
# expected to print a ``WARNING: ThreadSanitizer: data race`` report whose stack
27+
# traces point at ``gc_get_stats_impl`` (gcmodule.c) and ``gc_collect_main``
28+
# (gc_free_threading.c).
29+
#
30+
# Standalone usage:
31+
#
32+
# ./python Lib/test/test_free_threading/test_gc_get_stats_race.py
33+
#
34+
# or as part of the test suite (only meaningful under a TSAN build):
35+
#
36+
# ./python -m test test_free_threading.test_gc_get_stats_race
37+
38+
import gc
39+
import threading
40+
import unittest
41+
42+
from test.support import threading_helper
43+
44+
45+
# One thread hammers gc.collect() (the writer side); enough reader threads run
46+
# gc.get_stats() concurrently to make the race easy to observe. Readers run
47+
# until the writer is done so the test duration is controlled by the collection
48+
# count rather than by reader loop counts.
49+
NUM_READERS = 4
50+
ITERATIONS = 50
51+
52+
53+
def _stress_get_stats_race(num_readers=NUM_READERS, iterations=ITERATIONS):
54+
"""Race gc.collect() against gc.get_stats()."""
55+
56+
# Synchronise the start so the writer and readers overlap for as long as
57+
# possible, maximising the chance of the read and write landing on the
58+
# statistics struct at the same time.
59+
done = threading.Event()
60+
61+
def collector():
62+
try:
63+
for _ in range(iterations):
64+
# Writer: each full collection updates gcstate->generation_stats.
65+
gc.collect()
66+
finally:
67+
done.set()
68+
69+
def reader():
70+
while not done.is_set():
71+
# Reader: copies the per-generation stats structs with no lock.
72+
gc.get_stats()
73+
74+
threading_helper.run_concurrently([collector] + [reader] * num_readers)
75+
76+
77+
@threading_helper.requires_working_threading()
78+
class TestGCGetStatsRace(unittest.TestCase):
79+
def test_get_stats_collect_race(self):
80+
_stress_get_stats_race()
81+
82+
# The race is benign at the Python level: gc.get_stats() must still
83+
# return well-formed data and the interpreter must not crash.
84+
stats = gc.get_stats()
85+
self.assertEqual(len(stats), 3)
86+
for generation in stats:
87+
self.assertIn("collections", generation)
88+
self.assertIn("collected", generation)
89+
self.assertIn("uncollectable", generation)
90+
91+
92+
if __name__ == "__main__":
93+
# Standalone reproduction: run the race and exit cleanly so the script can
94+
# be reused as a regression check once the fix lands.
95+
print(f"Racing 1 gc.collect() thread against "
96+
f"{NUM_READERS} gc.get_stats() thread(s), {ITERATIONS} collections...")
97+
_stress_get_stats_race()
98+
print("Done (no Python-level crash). "
99+
"Run under a free-threading + TSAN build to observe the data race.")
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a data race in free-threading builds between :func:`gc.get_stats` and a
2+
concurrent garbage collection cycle. Access to the per-generation statistics
3+
is now serialized with a mutex so the reader observes a consistent snapshot.

Modules/gcmodule.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,15 @@ gc_get_stats_impl(PyObject *module)
374374
/* To get consistent values despite allocations while constructing
375375
the result list, we use a snapshot of the running stats. */
376376
GCState *gcstate = get_gc_state();
377+
#ifdef Py_GIL_DISABLED
378+
PyMutex_Lock(&gcstate->stats_mutex);
379+
#endif
377380
stats[0] = gcstate->generation_stats->young.items[gcstate->generation_stats->young.index];
378381
stats[1] = gcstate->generation_stats->old[0].items[gcstate->generation_stats->old[0].index];
379382
stats[2] = gcstate->generation_stats->old[1].items[gcstate->generation_stats->old[1].index];
383+
#ifdef Py_GIL_DISABLED
384+
PyMutex_Unlock(&gcstate->stats_mutex);
385+
#endif
380386

381387
PyObject *result = PyList_New(0);
382388
if (result == NULL)

Python/gc_free_threading.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2281,7 +2281,8 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
22812281
}
22822282
}
22832283

2284-
/* Update stats */
2284+
/* Update stats. */
2285+
PyMutex_Lock(&gcstate->stats_mutex);
22852286
struct gc_generation_stats *stats = get_stats(gcstate, generation);
22862287
stats->ts_start = start;
22872288
stats->ts_stop = stop;
@@ -2290,6 +2291,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
22902291
stats->uncollectable += n;
22912292
stats->duration += duration;
22922293
stats->candidates += state.candidates;
2294+
PyMutex_Unlock(&gcstate->stats_mutex);
22932295

22942296
GC_STAT_ADD(generation, objects_collected, m);
22952297
#ifdef Py_STATS

0 commit comments

Comments
 (0)