| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 8e6e75d commit 3453253
12 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -233,6 +233,7 @@ typedef struct _Py_DebugOffsets { | |||
| 233 | 233 | uint64_t debugger_script_path; | |
| 234 | 234 | uint64_t debugger_script_path_size; | |
| 235 | 235 | } debugger_support; | |
| 236 | + | ||
| 236 | 237 | } _Py_DebugOffsets; | |
| 237 | 238 | ||
| 238 | 239 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -178,27 +178,65 @@ struct gc_generation { | |||
| 178 | 178 | generations */ | |
| 179 | 179 | }; | |
| 180 | 180 | ||
| 181 | - struct gc_collection_stats { | ||
| 182 | - /* number of collected objects */ | ||
| 181 | + /* Stock 3.14's three counters. The generation_counters[] array below keeps | ||
| 182 | + this type and therefore its original 72 bytes at its original offset, so | ||
| 183 | + that _gc_runtime_state's layout is byte-identical to an unpatched build. | ||
| 184 | + It is still maintained, so an extension compiled against stock headers | ||
| 185 | + that reads it keeps reading correct values. */ | ||
| 186 | + struct gc_generation_counters { | ||
| 187 | + /* total number of collections */ | ||
| 188 | + Py_ssize_t collections; | ||
| 189 | + /* total number of collected objects */ | ||
| 183 | 190 | Py_ssize_t collected; | |
| 184 | 191 | /* total number of uncollectable objects (put into gc.garbage) */ | |
| 185 | 192 | Py_ssize_t uncollectable; | |
| 186 | 193 | }; | |
| 187 | 194 | ||
| 188 | 195 | /* Running stats per generation */ | |
| 189 | 196 | struct gc_generation_stats { | |
| 197 | + PyTime_t ts_start; | ||
| 198 | + PyTime_t ts_stop; | ||
| 190 | 199 | /* total number of collections */ | |
| 191 | 200 | Py_ssize_t collections; | |
| 192 | 201 | /* total number of collected objects */ | |
| 193 | 202 | Py_ssize_t collected; | |
| 194 | 203 | /* total number of uncollectable objects (put into gc.garbage) */ | |
| 195 | 204 | Py_ssize_t uncollectable; | |
| 205 | + // Total number of objects considered for collection and traversed: | ||
| 206 | + Py_ssize_t candidates; | ||
| 207 | + // Total duration of the collection in seconds: | ||
| 208 | + double duration; | ||
| 209 | + /* heap_size on the start of the collection */ | ||
| 210 | + Py_ssize_t heap_size; | ||
| 211 | + }; | ||
| 212 | + | ||
| 213 | + #ifdef Py_GIL_DISABLED | ||
| 214 | + #define GC_YOUNG_STATS_SIZE 1 | ||
| 215 | + #define GC_OLD_STATS_SIZE 1 | ||
| 216 | + #else | ||
| 217 | + #define GC_YOUNG_STATS_SIZE 11 | ||
| 218 | + #define GC_OLD_STATS_SIZE 3 | ||
| 219 | + #endif | ||
| 220 | + struct gc_young_stats_buffer { | ||
| 221 | + struct gc_generation_stats items[GC_YOUNG_STATS_SIZE]; | ||
| 222 | + int8_t index; | ||
| 223 | + }; | ||
| 224 | + | ||
| 225 | + struct gc_old_stats_buffer { | ||
| 226 | + struct gc_generation_stats items[GC_OLD_STATS_SIZE]; | ||
| 227 | + int8_t index; | ||
| 196 | 228 | }; | |
| 197 | 229 | ||
| 198 | 230 | /* If we change this, we need to change the default value in the | |
| 199 | 231 | signature of gc.collect. */ | |
| 200 | 232 | #define NUM_GENERATIONS 3 | |
| 201 | 233 | ||
| 234 | + struct gc_stats { | ||
| 235 | + struct gc_young_stats_buffer young; | ||
| 236 | + struct gc_old_stats_buffer old[2]; | ||
| 237 | + uint32_t update_seq; | ||
| 238 | + }; | ||
| 239 | + | ||
| 202 | 240 | struct _gc_runtime_state { | |
| 203 | 241 | /* List of objects that still need to be cleaned up, singly linked | |
| 204 | 242 | * via their gc headers' gc_prev pointers. */ | |
@@ -219,7 +257,7 @@ struct _gc_runtime_state { | |||
| 219 | 257 | ||
| 220 | 258 | /* a permanent generation which won't be collected */ | |
| 221 | 259 | struct gc_generation permanent_generation; | |
| 222 | - struct gc_generation_stats generation_stats[NUM_GENERATIONS]; | ||
| 260 | + struct gc_generation_counters generation_counters[NUM_GENERATIONS]; | ||
| 223 | 261 | /* true if we are currently running the collector */ | |
| 224 | 262 | int collecting; | |
| 225 | 263 | /* list of uncollectable objects */ | |
@@ -230,8 +268,15 @@ struct _gc_runtime_state { | |||
| 230 | 268 | /* The number of live objects. */ | |
| 231 | 269 | Py_ssize_t heap_size; | |
| 232 | 270 | ||
| 233 | - /* dummy members to preserve other offsets */ | ||
| 234 | - Py_ssize_t dummy1; /* was work_to_do */ | ||
| 271 | + /* Ring buffers of per-collection statistics, published to external | ||
| 272 | + readers through _Py_DebugOffsets. This occupies the slot 3.14 | ||
| 273 | + reserved as dummy1 when it reverted the incremental collector, so it | ||
| 274 | + costs no layout change: a pointer and a Py_ssize_t are both eight | ||
| 275 | + bytes with eight-byte alignment here. */ | ||
| 276 | + /* Upstream 3.15 spells this PyInterpreterState.gc.generation_stats | ||
| 277 | + and reaches it the same way; see the Provenance note in | ||
| 278 | + patches/gc-stats.patch for the one placement difference. */ | ||
| 279 | + struct gc_stats *generation_stats; /* was dummy1, was work_to_do */ | ||
| 235 | 280 | int dummy2; /* was visited_space */ | |
| 236 | 281 | int dummy3; /* was phase */ | |
| 237 | 282 | ||
@@ -260,6 +305,13 @@ struct _gc_runtime_state { | |||
| 260 | 305 | ||
| 261 | 306 | /* Mutex held for gc_should_collect_mem_usage(). */ | |
| 262 | 307 | PyMutex mutex; | |
| 308 | + /* Serializes gc.get_stats() (gc_get_stats_impl, the reader) against | ||
| 309 | + gc_collect_main()'s stats writer on the free-threaded build so a | ||
| 310 | + reader never observes a half-updated record (gh-151646). Placed | ||
| 311 | + after the existing mutex so it lands in this struct's trailing | ||
| 312 | + padding: no member moves and _gc_runtime_state stays byte-identical | ||
| 313 | + to a stock build. */ | ||
| 314 | + PyMutex stats_mutex; | ||
| 263 | 315 | #else | |
| 264 | 316 | PyGC_Head *generation0; | |
| 265 | 317 | #endif | |
| Original file line number | Diff line number | Diff 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.") | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -835,11 +835,15 @@ def test_get_stats(self): | |||
| 835 | 835 | self.assertEqual(len(stats), 3) | |
| 836 | 836 | for st in stats: | |
| 837 | 837 | self.assertIsInstance(st, dict) | |
| 838 | - self.assertEqual(set(st), | ||
| 839 | - {"collected", "collections", "uncollectable"}) | ||
| 838 | + self.assertEqual( | ||
| 839 | + set(st), | ||
| 840 | + {"collected", "collections", "uncollectable", "candidates", "duration"} | ||
| 841 | + ) | ||
| 840 | 842 | self.assertGreaterEqual(st["collected"], 0) | |
| 841 | 843 | self.assertGreaterEqual(st["collections"], 0) | |
| 842 | 844 | self.assertGreaterEqual(st["uncollectable"], 0) | |
| 845 | + self.assertGreaterEqual(st["candidates"], 0) | ||
| 846 | + self.assertGreaterEqual(st["duration"], 0) | ||
| 843 | 847 | # Check that collection counts are incremented correctly | |
| 844 | 848 | if gc.isenabled(): | |
| 845 | 849 | self.addCleanup(gc.enable) | |
@@ -850,6 +854,13 @@ def test_get_stats(self): | |||
| 850 | 854 | self.assertEqual(new[0]["collections"], old[0]["collections"] + 1) | |
| 851 | 855 | self.assertEqual(new[1]["collections"], old[1]["collections"]) | |
| 852 | 856 | self.assertEqual(new[2]["collections"], old[2]["collections"]) | |
| 857 | + self.assertGreater(new[0]["duration"], old[0]["duration"]) | ||
| 858 | + self.assertEqual(new[1]["duration"], old[1]["duration"]) | ||
| 859 | + self.assertEqual(new[2]["duration"], old[2]["duration"]) | ||
| 860 | + for stat in ["collected", "uncollectable", "candidates"]: | ||
| 861 | + self.assertGreaterEqual(new[0][stat], old[0][stat]) | ||
| 862 | + self.assertEqual(new[1][stat], old[1][stat]) | ||
| 863 | + self.assertEqual(new[2][stat], old[2][stat]) | ||
| 853 | 864 | gc.collect(2) | |
| 854 | 865 | new = gc.get_stats() | |
| 855 | 866 | self.assertEqual(new[0]["collections"], old[0]["collections"] + 1) | |
| Back | FazBrowse Home | New Git URL |
0 commit comments