seqlock overflow-consistency fix (#2866) is incomplete: two inline CPU-copy end-write paths still use non-wrapping `old_gen + 1`

[phil-opp]

⚠️ This issue was created by a scheduled, automated Claude code-review check. It was not filed by a human. Please verify the finding before acting on it. Severity is low (the overflow is practically unreachable); this is a code-consistency / completeness follow-up, not an active runtime risk.

Summary

Issue #2865 (fixed by #2866) asked for the memory-pool seqlock generation counter in apis/python/node/src/lib.rs to use wrapping_add consistently, to match seqlock_end_write's wrapping_add(2) and avoid a debug-build overflow panic. PR #2866 changed only seqlock_begin_write:

// seqlock_begin_write (fixed in #2866)
std::ptr::write_volatile(gen_ptr, pre_write_gen.wrapping_add(1));

However, the else (plain-CPU-copy) branch of write_memory_pool does not call seqlock_end_write; it inlines the end-write increment by hand, and both copies of that inlined code still use non-wrapping + 1. So the exact inconsistency #2865 was about still exists in two places in the same function.

Location

apis/python/node/src/lib.rs — the two inline "end write (infallible CPU copy)" blocks:

Fast path (~line 1800):

// Seqlock: end write (infallible CPU copy, always advances)
unsafe {
    let old_gen = std::ptr::read_volatile(gen_ptr);
    std::ptr::write_volatile(gen_ptr, old_gen + 1);   // <-- non-wrapping
    std::sync::atomic::fence(std::sync::atomic::Ordering::Release);
}

Slow path (~line 1945):

// Seqlock: end write (infallible CPU copy, always advances)
unsafe {
    let old_gen = std::ptr::read_volatile(gen_ptr);
    std::ptr::write_volatile(gen_ptr, old_gen + 1);   // <-- non-wrapping
    std::sync::atomic::fence(std::sync::atomic::Ordering::Release);
}

Why this matters

• Incomplete fix. Use wrapping_add(1) in seqlock_begin_write for overflow consistency #2865/fix(python): use wrapping_add(1) in seqlock_begin_write for overflow consistency #2866 aimed to make all generation-counter increments use uniform wrapping semantics. After fix(python): use wrapping_add(1) in seqlock_begin_write for overflow consistency #2866, seqlock_begin_write uses wrapping_add(1) and seqlock_end_write uses wrapping_add(2), but these two inline CPU-path increments were left as old_gen + 1. They are now the only non-wrapping increments of the same counter.
• Debug-build divergence. On overflow, old_gen + 1 panics in debug builds while every other seqlock increment wraps silently — the precise behavior Use wrapping_add(1) in seqlock_begin_write for overflow consistency #2865 called out.
• Severity is genuinely low. gen is u64 and advances by 2 per write; overflow requires ~2^63 writes, so this is not reachable in practice. This is a consistency/robustness cleanup, not a live bug.

Suggested fix

Either replace old_gen + 1 with old_gen.wrapping_add(1) in both inline blocks, or (cleaner, avoids future drift) have the CPU-copy path reuse seqlock_end_write(.., copy_ok = true) instead of hand-rolling the end-write increment, so there is a single end-write implementation.

How this was found

A scheduled automated review randomly selected apis/python/node/src/lib.rs, read the seqlock helpers, and cross-checked them against the recently-merged #2866. The function-level helper was fixed, but the two inline duplicates of the same logic were not.

[phil-opp]

Removed from the 1.0 milestone: the memory-pool transport is being parked out of the tree in #3152, so this no longer blocks 1.0.

The code is preserved under external/dora-pool, staged for lifting into its own repository. This bug is recorded in its README as a re-entry criterion — it must be fixed before the transport comes back. Not closing it: the defect is real and still unfixed.

[phil-opp]

Correction to my earlier note on this issue. The memory-pool transport is not being moved out of the repository after all.

It now lives at libraries/extensions/memory-pool as an opt-in feature (--features memory-pool, off by default) that is explicitly outside the 1.0 compatibility guarantees — see #3152. It reaches dora only through a new generic extension channel, so none of it is in dora's stable surface.

This stays off the 1.0 milestone for that reason, and stays open: the defect is real and unfixed. It is named in the extension's README as a known open defect.