FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[3.15] gh-155811: Add a seqcount to `gc_stats` to prevent torn reads (GH-155828) by maurycy · Pull Request #158829 · python/cpython · GitHub

Repository navigation

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .c  (3) .h  (1) .rst  (1) All 3 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
1 change: 1 addition & 0 deletions Include/internal/pycore_interp_structs.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.
42 changes: 36 additions & 6 deletions Modules/_remote_debugging/gc_stats.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 8 additions & 2 deletions Python/gc.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions Python/gc_free_threading.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -2282,6 +2282,12 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
}

/* Update 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 *stats = get_stats(gcstate, generation);
stats->ts_start = start;
stats->ts_stop = stop;
Expand All @@ -2290,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_release(&generation_stats->update_seq, seq + 2);

GC_STAT_ADD(generation, objects_collected, m);
#ifdef Py_STATS
Expand Down
Loading

Back | FazBrowse Home | New Git URL