diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index fe501be1afc59b1..9cc25eda44f9de1 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..23fa879b5032837 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 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 f865ac2f7db0853..42ebc1a2c0d77f5 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -2287,6 +2287,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; @@ -2295,6 +2301,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);