mosaicstack/stack
1
0
Fork 0
Code Issues 85 Pull Requests Actions Packages Projects Releases 15 Wiki Activity

wake/store enqueue TOCTOU (product bug, co-feed-live): pre-lock _wake_clean_stale_tmp deletes a concurrent enqueue's in-flight write → spurious durable-write-FAILED abort #927

New Issue
Closed
opened 2026-07-26 10:06:15 +00:00 by jason.woltje · 0 comments
jason.woltje commented 2026-07-26 10:06:15 +00:00
Owner
Copy Link

Genuine product-level TOCTOU race in the store enqueue path — surfaced (deterministically reproduced) while de-flaking T10 (#923/#926); NOT the test-timing gap #926 fixes, a real correctness bug. Elevated priority: co-feed is LIVE (detector + reconciler concurrently enqueue), so this is reachable in production now.

Mechanism. cmd_enqueue calls _wake_init_dir() at store.sh:166 — which invokes _wake_clean_stale_tmp() — BEFORE _wake_lock_acquire() at store.sh:174. _wake_clean_stale_tmp() unconditionally deletes EVERY .wake.tmp.* file in the state dir. So a second enqueue still in its pre-lock setup can delete the live in-flight temp write of a first enqueue that is holding the enqueue lock throughout its atomic write → the first enqueue's tmp→rename fails → a spurious "durable pending write FAILED" abort of a perfectly valid enqueue. Reproduced deterministically (a script holding the lock the whole time still had its live tmp deleted by a concurrent pre-lock cleanup), independent of scheduling luck. Likely explains part of the real CI flake history (spurious enqueue aborts under concurrency), beyond the T10 test-timing gap.

Impact. Under co-feed (now live post-vector), two concurrent enqueues can spuriously abort one — a false durability failure. It also intersects the #908 allocator's arrow-#1 guarantee (a failed durable write must not burn a seq): here the write "fails" spuriously, so no seq is burned (good, no gap), but a real obligation is dropped-on-the-floor at the enqueue (the caller sees a failed enqueue). For the detector that means the delta is retried next tick (recoverable); for the reconciler enumeration it depends on the caller's retry semantics — worth confirming no obligation is lost.

Fix (options). Move the stale-tmp cleanup INSIDE the enqueue lock (so no concurrent enqueue can clean during another's in-flight write); OR restrict _wake_clean_stale_tmp so it does NOT run on the hot enqueue path (run it only at daemon start / a maintenance path); OR make the cleanup skip tmp files that are recent / owned by a live pid. Preserve the intent (bounded stale-tmp accumulation) without racing a live write.

Red-first: two concurrent enqueues where one's (pre-fix) pre-lock cleanup WOULD clobber the other's in-flight write → post-fix BOTH succeed with distinct gapless seqs, ZERO spurious aborts (reproduce the abort RED against current code first). Refs: #892, #908 (allocator/enqueue seq-integrity), #923/#926 (surfaced here), #920 (co-feed live).

**Genuine product-level TOCTOU race in the store enqueue path** — surfaced (deterministically reproduced) while de-flaking T10 (#923/#926); NOT the test-timing gap #926 fixes, a real correctness bug. **Elevated priority: co-feed is LIVE (detector + reconciler concurrently enqueue), so this is reachable in production now.** **Mechanism.** `cmd_enqueue` calls `_wake_init_dir()` at `store.sh:166` — which invokes `_wake_clean_stale_tmp()` — BEFORE `_wake_lock_acquire()` at `store.sh:174`. `_wake_clean_stale_tmp()` unconditionally deletes EVERY `.wake.tmp.*` file in the state dir. So a second enqueue still in its **pre-lock setup** can delete the **live in-flight temp write** of a first enqueue that is holding the enqueue lock throughout its atomic write → the first enqueue's tmp→rename fails → a **spurious "durable pending write FAILED" abort** of a perfectly valid enqueue. Reproduced deterministically (a script holding the lock the whole time still had its live tmp deleted by a concurrent pre-lock cleanup), independent of scheduling luck. Likely explains part of the real CI flake history (spurious enqueue aborts under concurrency), beyond the T10 test-timing gap. **Impact.** Under co-feed (now live post-vector), two concurrent enqueues can spuriously abort one — a false durability failure. It also intersects the #908 allocator's arrow-#1 guarantee (a failed durable write must not burn a seq): here the write "fails" spuriously, so no seq is burned (good, no gap), but a real obligation is dropped-on-the-floor at the enqueue (the caller sees a failed enqueue). For the detector that means the delta is retried next tick (recoverable); for the reconciler enumeration it depends on the caller's retry semantics — worth confirming no obligation is lost. **Fix (options).** Move the stale-tmp cleanup INSIDE the enqueue lock (so no concurrent enqueue can clean during another's in-flight write); OR restrict `_wake_clean_stale_tmp` so it does NOT run on the hot enqueue path (run it only at daemon start / a maintenance path); OR make the cleanup skip tmp files that are recent / owned by a live pid. Preserve the intent (bounded stale-tmp accumulation) without racing a live write. **Red-first:** two concurrent enqueues where one's (pre-fix) pre-lock cleanup WOULD clobber the other's in-flight write → post-fix BOTH succeed with distinct gapless seqs, ZERO spurious aborts (reproduce the abort RED against current code first). Refs: #892, #908 (allocator/enqueue seq-integrity), #923/#926 (surfaced here), #920 (co-feed live).
Mos closed this issue 2026-07-26 10:56:41 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#927
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?