Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Include/internal/pycore_interp_structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
24 changes: 24 additions & 0 deletions Modules/_remote_debugging/gc_stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment thread
maurycy marked this conversation as resolved.
"Failed to read GC update sequence");
return -1;
}
if (_Py_RemoteDebug_ReadRemoteMemory(&offsets->handle,
gc_stats_addr,
sizeof(stats),
Expand All @@ -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");
Expand Down
9 changes: 7 additions & 2 deletions Python/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1399,6 +1399,12 @@ 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(&generation_stats->update_seq);
assert((seq & 1) == 0);
_Py_atomic_store_uint32(&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);

Expand All @@ -1412,9 +1418,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(&generation_stats->update_seq, seq + 2);
}

/* This is the main function. Read this to understand how the
Expand Down
6 changes: 6 additions & 0 deletions Python/gc_free_threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -2283,6 +2283,11 @@ 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(&generation_stats->update_seq);
assert((seq & 1) == 0);
_Py_atomic_store_uint32(&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;
Expand All @@ -2291,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(&generation_stats->update_seq, seq + 2);
PyMutex_Unlock(&gcstate->stats_mutex);

GC_STAT_ADD(generation, objects_collected, m);
Expand Down
Loading