fix(#332): re-entrancy guard on daemon-direct delivery + probe loops (storm-2) - #347
Merged
Conversation
Each deliveryTick does multiple slow cmux subprocess calls (findWorkspaceId + listSurfaces per project across ALL projects) and can exceed the 1s interval, so the bare setInterval could fire again while the previous tick was still in-flight. Two overlapping ticks read the SAME cursor seq and both delivered the entries after it -> duplicate/storm delivery, and the back-to-back cmux subprocess spawns saturated the daemon event loop (crew signal timeouts). Mirror the relay drain()'s 'draining' boolean: a closure-scoped guard set on entry, cleared in a finally, skipping overlapping fires. deliveryTick wraps deliveryCore with 'delivering'; probeTick (same hazard, slow pane reads on a 10s interval) gets its own independent 'probing' guard. Test: two overlapping tickDelivery() calls (send blocked on a controllable gate) deliver the entry exactly once; cursor advances once; a later tick does not re-deliver.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a SECOND notification storm found when live-testing daemon-direct (#332), plus the related control-plane-timeout.
Root cause: the daemon-direct delivery loop was
setInterval(() => deliveryTick(), 1000)with NO re-entrancy guard. Each tick does slow cmux subprocess calls (findWorkspaceId + listSurfaces per project) and can exceed 1s, so the next interval fires while the previous tick is mid-flight → overlapping ticks read the same cursor seq and both deliver the entries after it → duplicate/storm. It also saturated the daemon event loop (back-to-back cmux spawns), which is why a crew'scockpit crew signal donetimed out 3x.Fix: closure-scoped re-entrancy guards (
delivering/probing, set on entry, cleared infinally) on both the delivery loop and the blocked-crew probe loop — mirrors the relaydrain()'sdrainingguard.Parity audit: compared the daemon-direct delivery loop against the battle-tested relay
drain()for ALL safeguards (we'd already hit 4 storm bugs that were each a drain() safeguard the daemon loop lacked). Re-entrancy was the last missing one; everything else (stale-skip, per-seq defer via CaptainDelivery, advance-only-on-delivered, null-message gate) is at parity.break-vs-returnand the deliverable-gate location are correct multi-project adaptations, not bugs.Test: overlapping-tick test (gate-blocked send; tick #2 fires while tick #1 in-flight) asserts each entry delivered EXACTLY ONCE — RED before, GREEN after. Verification: build clean,
node dist/index.js --helploads, tsc clean, 39 tests pass (cockpitd-daemon-direct + relay-proxy + notify-relay, flag-OFF parity green).This is the 4th and (per the drain-parity audit) final storm safeguard. Daemon-direct still ships flag-OFF (relay = default per the re-scope); this hardens it for opt-in.