diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index 0623adce693d465..e370f52082939f5 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 000000000000000..2032fd74380db8b --- /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 d5d05edb8ecf5ee..e325b8bbc6bdb4d 100644 --- a/Modules/_remote_debugging/gc_stats.c +++ b/Modules/_remote_debugging/gc_stats.c @@ -103,6 +103,16 @@ get_gc_stats_from_interpreter_state(RuntimeOffsets *offsets, } struct gc_stats stats; + uintptr_t sequence_address = gc_stats_addr + + offsetof(struct gc_stats, update_seq); + 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), @@ -111,6 +121,20 @@ get_gc_stats_from_interpreter_state(RuntimeOffsets *offsets, 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)) { + PyErr_SetString(PyExc_RuntimeError, + "GC stats changed while being read; retry later"); + return -1; + } + if (read_gc_stats(&stats, iid, ctx->result, ctx->gc_stats_info_type) < 0) { set_exception_cause(offsets, PyExc_RuntimeError, "Failed to populate GC stats result"); diff --git a/Python/gc.c b/Python/gc.c index 201c621bcc3cb9b..bb20dae5a6543fa 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 99f1a1eb47e3ddc..5690aeb22495ac1 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -2283,6 +2283,12 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) /* Update stats. */ PyMutex_Lock(&gcstate->stats_mutex); + 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; @@ -2291,6 +2297,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); PyMutex_Unlock(&gcstate->stats_mutex); GC_STAT_ADD(generation, objects_collected, m);