gh-155811: Add a seqcount to gc_stats to prevent torn reads (#155828)

* update_seq

* no need for XCHGL, MOVL is enough?

* gh-155811: Retry an inconsistent GC snapshot once

---------

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>

Author: maurycy

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 {

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.

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,

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

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);