gh-155811: Add a seqcount to `gc_stats` to prevent torn reads by maurycy · Pull Request #155828 · python/cpython

maurycy · 2026-08-15T08:55:05Z

See #155811 for the context.

tl;dr get_gc_stats reads the stats without pausing the target (by design), so it can return torn data.

The PR adds a seqcount to gc_stats atomically increased by the writer around the stats update on every collection, where odd means it's in progress.

gcmon is the main consumer.

There's no retry, as per #155811 (comment). It's not needed. gc.collect() does not happen that often:

2026-08-15T10:53:48.504627000+0200 maurycy@gimel /Users/maurycy/work/cpython (gc-mon-seq 1291568?) % sudo ./python.exe gc_seq_collision.py 
successes=1966598 failures=191
GC stats changed while being read; retry later

For:

import subprocess
import sys
import time

import _remote_debugging

target = """
import gc
import os
import time

print(os.getpid(), flush=True)
while True:
    gc.collect(0)
    time.sleep(0.01)
"""

p = subprocess.Popen(
    [sys.executable, "-c", target],
    stdout=subprocess.PIPE,
    text=True,
)
try:
    pid = int(p.stdout.readline())
    monitor = _remote_debugging.GCMonitor(pid, debug=True)
    successes = 0
    failures = 0
    messages = set()
    deadline = time.monotonic() + 10.0
    while time.monotonic() < deadline:
        try:
            monitor.get_gc_stats(all_interpreters=False)
            successes += 1
        except RuntimeError as exc:
            failures += 1
            messages.add(str(exc))
    print(f"successes={successes} failures={failures}")
    for message in sorted(messages):
        print(message)
finally:
    p.terminate()
    p.wait()

I'm not sure what's optimal non-hacky test. We haven't had one in #152448. To be honest, it's implicitly tested by the current happy-path tests in test_gc_stats.

Issue: Torn reads of GC statistics from external process via get_gc_stats #155811

maurycy · 2026-08-15T08:55:28Z

cc @sergey-miryanov @nascheme

sergey-miryanov · 2026-08-15T11:42:34Z

I'm concerned that we now need to read memory three times with this approach:

struct gc_stats {
    uint32_t before_update_seq;
    struct gc_young_stats_buffer young;
    struct gc_old_stats_buffer old[2];
    uint32_t after_update_seq;
};

If before_update_seq and after_update_seq don't match, we have a torn read.

What do you think?

maurycy · 2026-08-15T11:53:00Z

@sergey-miryanov I'd push back. :-)

successes=1966598 failures=191

is around ~200KHz.

I'm not sure if there's a guarantee about copy-order and monotonicity of process_vm_readv etc.

Also, assuming these guarantees, before_update_seq and after_update_seq might be racy? What if:

Reader read before_update_seq (0)
Writer incremented before_update_seq (1)
Reader read after_update_seq (0)
Writer incremented after_update_seq (2)

sergey-miryanov · 2026-08-15T11:58:06Z

Yes, my variant will not work, I thought more about it - so drop it :)

sergey-miryanov · 2026-08-15T12:47:50Z

What if:
Writer:

Move update_seq to the begin of struct
Write to update_seq

Reader:

Read gc_stats (update_seq + stats)
Read update_seq
Compare update_seq(1) and update_seq(2)

sergey-miryanov · 2026-08-15T13:23:38Z

Even better, we could pack both counters into a single 32-bit integer — using the lower 16 bits for before and the upper 16 bits for after. This would let us read both values with a single atomic load.

What do you think?

maurycy · 2026-08-16T12:32:10Z

@sergey-miryanov The latter requires a full atomic snapshot read of the whole struct. Otherwise there's a race risk?

S = { (before=0, after=0), stats }

1. Reader read (before=0, after=0)...
2. Writer incremented `before=1`
3. Writer changed the stats
4. Reader read ...stats
5. Writer incremented `after=1`

Atomicity is not guaranteed:

       The data transfers performed by process_vm_readv() and
       process_vm_writev() are not guaranteed to be atomic in any way.

The former (ie: S = { update_seq, stats }) depends on a guarantee that update_seq is read before stats (copy-order / monotonicity, at least: target[0] is read before target[n > 0]), and I'm not sure if ReadProcessMemory or mach_vm_read_overwrite provide it. Windows does not document it, and macOS Mach VM seems to be sparsely documented.

Perhaps we could do _Py_RemoteDebug_BatchedReadRemoteMemory, since iovec provides such guarantee:

 Buffers are processed in array order.  This means that
 process_vm_readv() completely fills local_iov[0] before proceeding
 to local_iov[1], and so on.  Likewise, remote_iov[0] is completely
 read before proceeding to remote_iov[1], and so on.

That said, target[0] is read before target[n > 0] is likely how it works right now, but that's an implementation detail. I think that even memcpy on Linux/ARM breaks strong target[n] is read before target[n+1] and exactly-once read:

To be honest, I don't think that one additional syscall is that bad. Measured 200KHz is solid (way more than gcmon needs), it's not a hot path, and is trivial to wrap one's head around. In practice (implementation detail), I remember measuring it and small reads are much faster.

I also added update_seq at the end to avoid breaking clients. Not sure if this matters but I remember this comment.

The thing that I think is worth measuring - and I'm not an expert here - is the probability of collision. What's the maximum observable GC? The time.sleep(0.1) is 100Hz. Can we expect the rate of 100Khz GC collections?

Sorry for so much text :) I keep posting essays here but wanted to explain myself, since maybe I'm making a fatal blunder here. Does it make sense to you?

sergey-miryanov · 2026-08-16T16:00:58Z

I tested on this script:

import gc
import time

ts_start = time.perf_counter_ns()
stats_start = gc.get_stats()
while True:
    try:
        gc.collect(2)
    except KeyboardInterrupt:
        break

ts_stop = time.perf_counter_ns()
stats_stop = gc.get_stats()

dt = (ts_stop - ts_start) / 1e9
dc = (stats_stop[2]["collections"] - stats_start[2]["collections"])
dd = (stats_stop[2]["duration"] - stats_start[2]["duration"])

print("collections", dc)
print(f"durations: {dd:.4f}, rate: {dc/dd:.2f}, avg: {dd/dc:.6f}")
print(f"time: {dt:.4f}, rate: {dc/dt:.2f}, avg: {dt/dc:.6f}")

Average duration for gc.collect(2) is about 0.210 ms on my machine (11th Gen Intel(R) Core(TM) i5-11600K @ 3.90GHz, Windows 11)

> python .\empty_gc.py
collections 17523
durations: 3.7099, rate: 4723.37, avg: 0.000212
time: 3.7173, rate: 4713.90, avg: 0.000212

Average duration of _remote_debugging.get_gc_stats is about 0.560 ms.

| PID:IID | Metric      |     Count |              Sum |   Avg |   P50 |   P90 |   P95 |   P99 |    Cov |      F |
|---------|-------------|-----------|------------------|-------|-------|-------|-------|-------|--------|--------|
| Total   | GC Pause(0) |         3 |            0.286 | 0.095 | 0.094 | 0.102 | 0.103 | 0.104 | 100.0% |  1.000 |
|         | GC Pause(2) | 804/19676 | 181.871/4180.404 | 0.226 | 0.225 | 0.236 | 0.241 | 0.294 |   4.1% | 22.985 |
|---------|-------------|-----------|------------------|-------|-------|-------|-------|-------|--------|--------|
|         | Read Time   |       268 |          148.984 | 0.556 | 0.546 | 0.576 | 0.622 | 0.790 |        |        |

I don't think we can achieve 100Khz rate for gc.collect :)

maurycy · 2026-08-31T12:50:14Z

@colesbury I know that we've just removed _PySeqLock but I'd love your sanity check here! Thank you.

sergey-miryanov · 2026-09-09T07:49:41Z

I don't think we should retry more than once if the GC stats read is inconsistent.

I mean one more try if we fail on the first attempt. @maurycy, would you mind adding a single retry pass?

maurycy · 2026-10-04T19:37:03Z

I don't think we should retry more than once if the GC stats read is inconsistent.

I mean one more try if we fail on the first attempt. @maurycy, would you mind adding a single retry pass?

@sergey-miryanov It introduces some complexity. Do you think it could be avoided with gcmon retrying?

sergey-miryanov · 2026-10-04T21:17:46Z

@maurycy It seems cheaper to do the retry inside the call rather than from gcmon.

A GCMonitor.get_gc_stats call costs about 30μs on my machine right now. But with the new GC, we can have pauses as short as ~200ns with ~100μs intervals between them.

pablogsal

LGTM!

maurycy · 2026-10-05T04:55:49Z

That was faster than light. Thank you, @pablogsal. 🖤

miss-islington-app · 2026-10-05T07:47:32Z

Thanks @maurycy for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington-app · 2026-10-05T07:47:39Z

Sorry, @maurycy and @pablogsal, I could not cleanly backport this to 3.15 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker 5fecd448bb120378978a37dde65dfce233d88c0d 3.15

sergey-miryanov · 2026-10-05T07:47:48Z

Thanks all!

sergey-miryanov · 2026-10-05T07:51:52Z

@maurycy Do you want to take a look at the backport? If not, I can handle it myself.

bedevere-app · 2026-10-05T08:14:13Z

GH-158829 is a backport of this pull request to the 3.15 branch.

maurycy · 2026-10-05T08:16:09Z

@sergey-miryanov #158829

bedevere-app · 2026-10-05T08:34:40Z

GH-158829 is a backport of this pull request to the 3.15 branch.

…H-155828) (#158829) * update_seq * no need for XCHGL, MOVL is enough? * gh-155811: Retry an inconsistent GC snapshot once --------- (cherry picked from commit 5fecd44) Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…nt torn reads (pythonGH-155828) (python#158829)" This reverts commit 0281240.

…58882) * [3.15] gh-153364: Make frame, coroutine, and task-waiter chain walks iterative and bounded (GH-153365) (#158813) gh-153364: Make frame, coroutine, and task-waiter chain walks iterative and bounded (GH-153365) * let me declare single limit * use our new limit in process_frame_chain() * add it in parse_async_frame_chain() * parse_coro_chain() * NEWS * async in the message? * test * no race * process_task_awaited_by * process_task_awaited_by limit test * NEWS * MAX_TASK_WAITER_CHAIN_DEPTH * TASK_WAITER_CHAIN_DEPTH in test * TASK_WAITER_CHAIN_DEPTH 256 * prevent the drift with the comment * better naming, better style * MAX_TASK_WAITER_CHAIN_DEPTH comment * task-waiter iterative bfs walk * iterative coro-walk * nicer news * 1 << 14 * comment * unused read_Py_ssize_t * fix tombstones * simplify * correct msg * better test * news for tombstones * left-over from when testing buggy version * redundant new line (cherry picked from commit e0861c6) Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com> (cherry picked from commit 9e401cf) * [3.15] gh-155811: Add a seqcount to `gc_stats` to prevent torn reads (GH-155828) (#158829) * update_seq * no need for XCHGL, MOVL is enough? * gh-155811: Retry an inconsistent GC snapshot once --------- (cherry picked from commit 5fecd44) Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 0281240) * [3.15] gh-151292: `_remote_debugging`: Do not corrupt the binary file when hitting `OverflowError` (GH-152892) (#158830) gh-151292: `_remote_debugging`: Do not corrupt the binary file when hitting `OverflowError` (GH-152892) * the kolektor * test * better test * news * =Q, move const to the base, not self.running * gh-151292: Track binary writer finalization state --------- (cherry picked from commit f839c06) Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com> Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com> (cherry picked from commit 8a7c23f) * [3.15] gh-158583: Fix uninitialized memory read in bytes.fromhex() (GH-158584) (#158691) gh-158583: Fix uninitialized memory read in bytes.fromhex() (GH-158584) (cherry picked from commit 9d22a53) Co-authored-by: Victor Stinner <vstinner@python.org> (cherry picked from commit 4f7af46) * [3.15] gh-154194: Degrade frames in Tachyon instead of failing the sample (GH-154195) (#158831) * gh-154194: Degrade frames in Tachyon instead of failing the sample (#154195) * degrade gracefully * news * better NEWS wording * do not raise on MAX_REMOTE_STR_READ * bye MAX_REMOTE_STR_READ * fix -m asyncio ps|pstree * test truncation and linetable sentinel * simpler * simpler * redundant now * respect #157790 in the news --------- Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com> (cherry picked from commit 7d25916) * Preserve the stable ABI when creating fallback frame names --------- Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com> (cherry picked from commit c27f494) * [3.15] Add MSan to CI (GH-158625) (#158832) Run a job that the test suite with MSan to the CI (#158625) * Run the test suite with MSan in CI * Additional fixes * Add `_Py_MSAN_UNPOISON_STRING` * Apply Victor's suggestions * Apply Victor's suggestions --------- (cherry picked from commit b93fb19) Co-authored-by: Stan Ulbrych <stan@python.org> Co-authored-by: Victor Stinner <victor.stinner@gmail.com> (cherry picked from commit 1bc78de) * [3.15] gh-156810: Write the profiler's collapsed-stack export as UTF-8 (GH-156811) (#156814) gh-156810: Write the profiler's collapsed-stack export as UTF-8 (GH-156811) (cherry picked from commit c3706f4) Co-authored-by: tonghuaroot (童话) <tonghuaroot@gmail.com> (cherry picked from commit fce28da) * [3.15] gh-158552: Wait for Windows threads to suspend before blocking sampling (GH-158802) (#158845) gh-158552: Wait for Windows threads to suspend before blocking sampling (GH-158802) * gh-158552: Wait for Windows threads to suspend before blocking sampling * Use a named Windows thread enumeration status constant (cherry picked from commit 1643525) Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com> (cherry picked from commit 31288da) * [3.15] gh-152721: Fix quadratic RLE replay time in the profiling binary reader (GH-152722) (#158850) Backport of GH-152722. Co-authored-by: tonghuaroot <tonghuaroot@gmail.com> (cherry picked from commit ebaca2a) * [3.15] gh-156545: Fix flamegraph export RecursionError on deeply recursive programs (GH-156546) (#158851) Backport of GH-156546. Co-authored-by: tonghuaroot (童话) <tonghuaroot@gmail.com> (cherry picked from commit 0ac7217) * [3.15] gh-158540: Add the profiled script's directory to sys.path (GH-158548) (#158844) gh-158540: Add the profiled script's directory to sys.path (GH-158548) * gh-158540: Add the profiled script's directory to sys.path When a script is profiled with ``python -m profiling.sampling run script.py`` from another directory, the script cannot import modules placed next to it, because ``_sync_coordinator._execute_script()`` executes it with the working directory (added by ``_setup_environment()`` for the module case) as ``sys.path[0]`` instead of the script's own directory. Make the script's directory importable in ``_execute_script()``, matching the behavior of ``python script.py``. Add a regression test that runs the coordinator on a script importing a sibling module. * Update _sync_coordinator.py Comment simplified * gh-158540: Resolve symlinks when adding the script directory to sys.path ``python script.py`` resolves symlinks when computing ``sys.path[0]``, so a script reached through a symlink (``link.py -> sub/where.py``) imports modules from the real script's directory. Apply ``os.path.realpath()`` before taking the directory name, and make sure the result is placed at the front of ``sys.path`` even if it was already listed. Add a regression test for a symlinked script. --------- (cherry picked from commit 3f02aab) Co-authored-by: he_tao <53343436+hetaozdh@users.noreply.github.com> Co-authored-by: Eduardo Villalpando Mello <eduardo.villalpando.mello@gmail.com> (cherry picked from commit f52d831) * [3.15] gh-153838: Skip non-regular source files in the heatmap exporter (GH-153839) (#158853) Backport of GH-153839. Co-authored-by: tonghuaroot <tonghuaroot@gmail.com> (cherry picked from commit 48998df) * [3.15] gh-158539: Fix exception mode missing handlers in generators/coroutines (GH-158581) (#158852) * [3.15] gh-158539: Fix exception mode missing handlers in generators/coroutines (GH-158581) Backport of GH-158581. Co-authored-by: LucasZhou <donghao.zhou@outlook.com> * [3.15] gh-158539: Use portable static assertion messages * [3.15] gh-158539: Keep layout assertions with debug-offset validation * [3.15] gh-158539: Use the platform guard for in-process inspection tests --------- Co-authored-by: LucasZhou <donghao.zhou@outlook.com> (cherry picked from commit d625ecb) * [3.15] gh-158522: Fix truncated stack for a task whose coroutine recurses (GH-158526) (#158870) Co-authored-by: Timofei Ivankov <128279579+deadlovelll@users.noreply.github.com> (cherry picked from commit a904b39) * gh-156545, gh-158539: Fix deep flamegraph export on small C stacks and macOS runtime lookup (#158874) (cherry picked from commit 114de19) Include the C-stack test helper from main, introduced by ce5ae29, which the regression test requires but 3.15 does not yet provide. --------- Co-authored-by: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Stan Ulbrych <stan@python.org> Co-authored-by: Victor Stinner <victor.stinner@gmail.com> Co-authored-by: tonghuaroot (童话) <tonghuaroot@gmail.com> Co-authored-by: he_tao <53343436+hetaozdh@users.noreply.github.com> Co-authored-by: Eduardo Villalpando Mello <eduardo.villalpando.mello@gmail.com> Co-authored-by: LucasZhou <donghao.zhou@outlook.com> Co-authored-by: Timofei Ivankov <128279579+deadlovelll@users.noreply.github.com>

update_seq

1291568

maurycy requested a review from pablogsal as a code owner August 15, 2026 08:55

bedevere-app Bot added the awaiting review label Aug 15, 2026

bedevere-app Bot mentioned this pull request Aug 15, 2026

Torn reads of GC statistics from external process via get_gc_stats #155811

Closed

maurycy commented Aug 15, 2026

View reviewed changes

Comment thread Modules/_remote_debugging/gc_stats.c Outdated

no need for XCHGL, MOVL is enough?

0cb2510

This was referenced Aug 19, 2026

_remote_debugging: get_child_pids() iterates all processes four times #156060

Closed

Attach to each process once, instead of on every poll sergey-miryanov/gcmon#107

Merged

Merge branch 'main' into gc-mon-seq

424b95f

pythongh-155811: Retry an inconsistent GC snapshot once

577305e

pablogsal approved these changes Oct 5, 2026

View reviewed changes

pablogsal enabled auto-merge (squash) October 5, 2026 01:17

bedevere-app Bot added awaiting merge and removed awaiting review labels Oct 5, 2026

pablogsal merged commit 5fecd44 into python:main Oct 5, 2026
59 checks passed

bedevere-app Bot removed the awaiting merge label Oct 5, 2026

sergey-miryanov added the needs backport to 3.15 bugs and security fixes label Oct 5, 2026

miss-islington-app Bot assigned pablogsal Oct 5, 2026

maurycy mentioned this pull request Oct 5, 2026

[3.15] gh-155811: Add a seqcount to gc_stats to prevent torn reads (GH-155828) #158829

Merged

bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 5, 2026

maurycy deleted the gc-mon-seq branch October 5, 2026 08:35

pablogsal added a commit to pablogsal/cpython that referenced this pull request Oct 5, 2026

Revert "[3.15] pythongh-155811: Add a seqcount to gc_stats to preve…

693eccc

…nt torn reads (pythonGH-155828) (python#158829)" This reverts commit 0281240.

Conversation

maurycy commented Aug 15, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maurycy commented Aug 15, 2026

Uh oh!

Uh oh!

sergey-miryanov commented Aug 15, 2026

Uh oh!

maurycy commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-miryanov commented Aug 15, 2026

Uh oh!

sergey-miryanov commented Aug 15, 2026

Uh oh!

sergey-miryanov commented Aug 15, 2026

Uh oh!

maurycy commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-miryanov commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maurycy commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-miryanov commented Sep 9, 2026

Uh oh!

maurycy commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-miryanov commented Oct 4, 2026

Uh oh!

pablogsal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maurycy commented Oct 5, 2026

Uh oh!

miss-islington-app Bot commented Oct 5, 2026

Uh oh!

miss-islington-app Bot commented Oct 5, 2026

Uh oh!

sergey-miryanov commented Oct 5, 2026

Uh oh!

sergey-miryanov commented Oct 5, 2026

Uh oh!

bedevere-app Bot commented Oct 5, 2026

Uh oh!

maurycy commented Oct 5, 2026

Uh oh!

bedevere-app Bot commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maurycy commented Aug 15, 2026 •

edited by bedevere-app Bot

Loading

maurycy commented Aug 15, 2026 •

edited

Loading

maurycy commented Aug 16, 2026 •

edited

Loading

sergey-miryanov commented Aug 16, 2026 •

edited

Loading

maurycy commented Aug 31, 2026 •

edited

Loading

maurycy commented Oct 4, 2026 •

edited

Loading