Skip to content

Commit 1291568

Browse files
committed
update_seq
1 parent bc6749c commit 1291568

5 files changed

Lines changed: 41 additions & 2 deletions

File tree

Include/internal/pycore_interp_structs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ struct gc_old_stats_buffer {
219219
struct gc_stats {
220220
struct gc_young_stats_buffer young;
221221
struct gc_old_stats_buffer old[2];
222+
uint32_t update_seq;
222223
};
223224

224225
struct _gc_runtime_state {
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Add a sequence counter to GC statistics to prevent :mod:`!_remote_debugging`
2+
returning inconsistent snapshots caused by non-atomic reads. Patch by Maurycy
3+
Pawłowski-Wieroński.

Modules/_remote_debugging/gc_stats.c

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,16 @@ get_gc_stats_from_interpreter_state(RuntimeOffsets *offsets,
103103
}
104104

105105
struct gc_stats stats;
106+
uintptr_t sequence_address = gc_stats_addr
107+
+ offsetof(struct gc_stats, update_seq);
108+
uint32_t before;
109+
if (_Py_RemoteDebug_ReadRemoteMemory(&offsets->handle,
110+
sequence_address,
111+
sizeof(before), &before) < 0) {
112+
set_exception_cause(offsets, PyExc_RuntimeError,
113+
"Failed to read GC update sequence");
114+
return -1;
115+
}
106116
if (_Py_RemoteDebug_ReadRemoteMemory(&offsets->handle,
107117
gc_stats_addr,
108118
sizeof(stats),
@@ -111,6 +121,20 @@ get_gc_stats_from_interpreter_state(RuntimeOffsets *offsets,
111121
return -1;
112122
}
113123

124+
uint32_t after;
125+
if (_Py_RemoteDebug_ReadRemoteMemory(&offsets->handle,
126+
sequence_address,
127+
sizeof(after), &after) < 0) {
128+
set_exception_cause(offsets, PyExc_RuntimeError,
129+
"Failed to read GC update sequence");
130+
return -1;
131+
}
132+
if (before != after || before != stats.update_seq || (after & 1)) {
133+
PyErr_SetString(PyExc_RuntimeError,
134+
"GC stats changed while being read; retry later");
135+
return -1;
136+
}
137+
114138
if (read_gc_stats(&stats, iid, ctx->result,
115139
ctx->gc_stats_info_type) < 0) {
116140
set_exception_cause(offsets, PyExc_RuntimeError, "Failed to populate GC stats result");

Python/gc.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,6 +1399,12 @@ gc_get_prev_stats(GCState *gcstate, int gen)
13991399
static void
14001400
add_stats(GCState *gcstate, int gen, struct gc_generation_stats *stats)
14011401
{
1402+
struct gc_stats *generation_stats = gcstate->generation_stats;
1403+
uint32_t seq = _Py_atomic_load_uint32(&generation_stats->update_seq);
1404+
assert((seq & 1) == 0);
1405+
_Py_atomic_store_uint32(&generation_stats->update_seq, seq + 1);
1406+
_Py_atomic_fence_seq_cst();
1407+
14021408
struct gc_generation_stats *prev_stats = gc_get_prev_stats(gcstate, gen);
14031409
struct gc_generation_stats *cur_stats = gc_get_stats(gcstate, gen);
14041410

@@ -1412,9 +1418,8 @@ add_stats(GCState *gcstate, int gen, struct gc_generation_stats *stats)
14121418

14131419
cur_stats->duration += stats->duration;
14141420
cur_stats->heap_size = stats->heap_size;
1415-
/* Publish ts_stop last so remote readers do not select a partially
1416-
updated stats record as the latest collection. */
14171421
cur_stats->ts_stop = stats->ts_stop;
1422+
_Py_atomic_store_uint32(&generation_stats->update_seq, seq + 2);
14181423
}
14191424

14201425
/* This is the main function. Read this to understand how the

Python/gc_free_threading.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2283,6 +2283,11 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
22832283

22842284
/* Update stats. */
22852285
PyMutex_Lock(&gcstate->stats_mutex);
2286+
struct gc_stats *generation_stats = gcstate->generation_stats;
2287+
uint32_t seq = _Py_atomic_load_uint32(&generation_stats->update_seq);
2288+
assert((seq & 1) == 0);
2289+
_Py_atomic_store_uint32(&generation_stats->update_seq, seq + 1);
2290+
_Py_atomic_fence_seq_cst();
22862291
struct gc_generation_stats *stats = get_stats(gcstate, generation);
22872292
stats->ts_start = start;
22882293
stats->ts_stop = stop;
@@ -2291,6 +2296,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
22912296
stats->uncollectable += n;
22922297
stats->duration += duration;
22932298
stats->candidates += state.candidates;
2299+
_Py_atomic_store_uint32(&generation_stats->update_seq, seq + 2);
22942300
PyMutex_Unlock(&gcstate->stats_mutex);
22952301

22962302
GC_STAT_ADD(generation, objects_collected, m);

0 commit comments

Comments
 (0)