https://github.com/python/cpython/commit/028124039c276f73fbc26036f1629c91be4c2891 commit: 028124039c276f73fbc26036f1629c91be4c2891 branch: 3.15 author: Maurycy Pawłowski-Wieroński <[email protected]> committer: pablogsal <[email protected]> date: 2026-10-05T10:21:24+01:00 summary:
[3.15] gh-155811: Add a seqcount to `gc_stats` to prevent torn reads (GH-155828) (#158829) * update_seq * no need for XCHGL, MOVL is enough? * gh-155811: Retry an inconsistent GC snapshot once --------- (cherry picked from commit 5fecd448bb120378978a37dde65dfce233d88c0d) Co-authored-by: Pablo Galindo Salgado <[email protected]> Co-authored-by: Claude Fable 5.1 <[email protected]> files: A Misc/NEWS.d/next/Library/2026-08-15-10-20-40.gh-issue-155811.knP-YB.rst M Include/internal/pycore_interp_structs.h M Modules/_remote_debugging/gc_stats.c M Python/gc.c M Python/gc_free_threading.c diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index 58a15eb87d2aad..36d05efc4ce4b6 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -219,6 +219,7 @@ struct gc_old_stats_buffer { struct gc_stats { struct gc_young_stats_buffer young; struct gc_old_stats_buffer old[2]; + uint32_t update_seq; }; struct _gc_runtime_state { diff --git a/Misc/NEWS.d/next/Library/2026-08-15-10-20-40.gh-issue-155811.knP-YB.rst b/Misc/NEWS.d/next/Library/2026-08-15-10-20-40.gh-issue-155811.knP-YB.rst new file mode 100644 index 00000000000000..2032fd74380db8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-08-15-10-20-40.gh-issue-155811.knP-YB.rst @@ -0,0 +1,3 @@ +Add a sequence counter to GC statistics to prevent :mod:`!_remote_debugging` +returning inconsistent snapshots caused by non-atomic reads. Patch by Maurycy +Pawłowski-Wieroński. diff --git a/Modules/_remote_debugging/gc_stats.c b/Modules/_remote_debugging/gc_stats.c index d5d05edb8ecf5e..23fa879b503283 100644 --- a/Modules/_remote_debugging/gc_stats.c +++ b/Modules/_remote_debugging/gc_stats.c @@ -103,12 +103,42 @@ get_gc_stats_from_interpreter_state(RuntimeOffsets *offsets, } struct gc_stats stats; - if (_Py_RemoteDebug_ReadRemoteMemory(&offsets->handle, - gc_stats_addr, - sizeof(stats), - &stats) < 0) { - set_exception_cause(offsets, PyExc_RuntimeError, "Failed to read GC state"); - return -1; + uintptr_t sequence_address = gc_stats_addr + + offsetof(struct gc_stats, update_seq); + /* A short GC update may finish before a second attempt. */ + for (int attempt = 0; attempt < 2; attempt++) { + uint32_t before; + if (_Py_RemoteDebug_ReadRemoteMemory(&offsets->handle, + sequence_address, + sizeof(before), &before) < 0) { + set_exception_cause(offsets, PyExc_RuntimeError, + "Failed to read GC update sequence"); + return -1; + } + if (_Py_RemoteDebug_ReadRemoteMemory(&offsets->handle, + gc_stats_addr, + sizeof(stats), + &stats) < 0) { + set_exception_cause(offsets, PyExc_RuntimeError, "Failed to read GC state"); + return -1; + } + + uint32_t after; + if (_Py_RemoteDebug_ReadRemoteMemory(&offsets->handle, + sequence_address, + sizeof(after), &after) < 0) { + set_exception_cause(offsets, PyExc_RuntimeError, + "Failed to read GC update sequence"); + return -1; + } + if (before == after && before == stats.update_seq && !(after & 1)) { + break; + } + if (attempt == 1) { + PyErr_SetString(PyExc_RuntimeError, + "GC stats changed while being read; retry later"); + return -1; + } } if (read_gc_stats(&stats, iid, ctx->result, diff --git a/Python/gc.c b/Python/gc.c index 201c621bcc3cb9..bb20dae5a6543f 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -1399,6 +1399,13 @@ gc_get_prev_stats(GCState *gcstate, int gen) static void add_stats(GCState *gcstate, int gen, struct gc_generation_stats *stats) { + struct gc_stats *generation_stats = gcstate->generation_stats; + uint32_t seq = _Py_atomic_load_uint32_relaxed(&generation_stats->update_seq); + assert((seq & 1) == 0); + /* Odd seq tells the reader that an update is in progress. */ + _Py_atomic_store_uint32_relaxed(&generation_stats->update_seq, seq + 1); + _Py_atomic_fence_seq_cst(); + struct gc_generation_stats *prev_stats = gc_get_prev_stats(gcstate, gen); struct gc_generation_stats *cur_stats = gc_get_stats(gcstate, gen); @@ -1412,9 +1419,8 @@ add_stats(GCState *gcstate, int gen, struct gc_generation_stats *stats) cur_stats->duration += stats->duration; cur_stats->heap_size = stats->heap_size; - /* Publish ts_stop last so remote readers do not select a partially - updated stats record as the latest collection. */ cur_stats->ts_stop = stats->ts_stop; + _Py_atomic_store_uint32_release(&generation_stats->update_seq, seq + 2); } /* This is the main function. Read this to understand how the diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 8e27649bfd6941..f408f239ab1693 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -2282,6 +2282,12 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) } /* Update stats */ + struct gc_stats *generation_stats = gcstate->generation_stats; + uint32_t seq = _Py_atomic_load_uint32_relaxed(&generation_stats->update_seq); + assert((seq & 1) == 0); + /* Odd seq tells the reader that an update is in progress. */ + _Py_atomic_store_uint32_relaxed(&generation_stats->update_seq, seq + 1); + _Py_atomic_fence_seq_cst(); struct gc_generation_stats *stats = get_stats(gcstate, generation); stats->ts_start = start; stats->ts_stop = stop; @@ -2290,6 +2296,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) stats->uncollectable += n; stats->duration += duration; stats->candidates += state.candidates; + _Py_atomic_store_uint32_release(&generation_stats->update_seq, seq + 2); GC_STAT_ADD(generation, objects_collected, m); #ifdef Py_STATS _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
