You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
relay: the CLOSE-during-mint test synchronises on a 50 ms sleep and can fail under load #963
a_close_during_the_invite_mint_leaves_the_event_but_not_the_read (crates/nostream-relay/tests/handlers.rs:1135, added in #962) synchronises the CLOSE against the invite mint with a fixed sleep:
gate.add_permits(1);
...is preceded only by tokio::time::sleep(Duration::from_millis(50)) after ["CLOSE","invite"] is sent. Nothing confirms the server has processed the CLOSE — i.e. that unsubscribe::handle has run abort_read and set the in-flight read's cancellation — before the gate releases the mint.
Failure mode: a false failure, not a false pass. If a busy runner delays the CLOSE handler past the sleep, the mint resumes first, the handler reaches fetch_and_send with the cancellation not yet set, and the test then observes the stored EVENT and the EOSE it asserts must not arrive. The fix is correct and the test goes red anyway. The window is small (50 ms against a local in-memory store) and the test passed in CI on #962, so this is a flake risk rather than a live defect.
Suggested fix. Wait for a server-side signal instead of for elapsed time. The crate already has the passive half — Connection::read_in_flight(id) is observable — so the options are:
Poll fixture.connection(id).read_in_flight("invite") == false (the abort removes the entry) before gate.add_permits(1), with a bounded timeout that fails with a clear message rather than hanging.
Add an observable for "a read was aborted" on the test double, mirroring the existing gate_reads / gate_reads_per_item / gate_creates idiom in testing.rs, and await it.
Either way the release becomes ordered after the abort by construction, and the 50 ms sleep goes.
Residual risk worth stating in the fix: the CLOSE's handler runs in a spawned task (relay.rs spawns each client message independently), so a wait on server-side state is the only ordering that does not depend on scheduling. That is also the mechanism behind #958, which is a separate pre-existing defect and not this test's problem.
Found by the Greptile review on #962 (P2). Recorded rather than fixed there, per the rule that a bot finding which is not a showstopper becomes its own issue instead of blocking the merge that revealed it.
Fixed by #987 (branch factory/014-close-mint-sync, based on rewrite/main @ 1cb6265, awaiting merge).
The false failure was real and reproduced: a 200 ms delay injected into the Unsubscribe dispatch makes the old test fail exactly as described. The natural run does not reproduce it (10 oversubscribed runs, 0 failures), so the risk was real but not observable without the injection.
Neither fix this issue proposes was test-only — there is no fixture→Connection path at that head, and no double sits on the abort path. The shape that is test-only is a handler-level rewrite, and that is what landed: the test owns a Connection, calls the synchronous unsubscribe::handle in its own task, asserts !read_in_flight("invite")before releasing the gate, then checks the frames. The sleep is gone rather than bounded; the ordering is program order.
What that cost: the invite-EVENT-plus-CLOSE-mid-mint combination is no longer covered over a socket. Tracked as #983 with the shape a production-hook fix needs, so the gap is recorded rather than discovered later.
How it is proved: a mutation — abort_read moved after reading.await — fails at a closed subscription streams nothing with the stored kind-1 event in the sink. The 200 ms injection cannot discriminate the new test (it never enters handlers::dispatch) and the job's build.md says so rather than quoting a green as evidence.
Activity
docs(relay): say what the abort guard in the CLOSE test actually catc…
test(relay): order the CLOSE-during-mint test by program order (#963)
docs(relay): say what the abort guard in the CLOSE test actually catc…
test(relay): spawn subscribe::begin in the CLOSE-during-mint test (#963)
docs(relay): name subscribe::begin in the changeset (#963)
chappie-daemon commentedon Oct 25, 2026
Fixed by #987 (branch
factory/014-close-mint-sync, based onrewrite/main@1cb6265, awaiting merge).The false failure was real and reproduced: a 200 ms delay injected into the
Unsubscribedispatch makes the old test fail exactly as described. The natural run does not reproduce it (10 oversubscribed runs, 0 failures), so the risk was real but not observable without the injection.Neither fix this issue proposes was test-only — there is no fixture→
Connectionpath at that head, and no double sits on the abort path. The shape that is test-only is a handler-level rewrite, and that is what landed: the test owns aConnection, calls the synchronousunsubscribe::handlein its own task, asserts!read_in_flight("invite")before releasing the gate, then checks the frames. The sleep is gone rather than bounded; the ordering is program order.What that cost: the invite-
EVENT-plus-CLOSE-mid-mint combination is no longer covered over a socket. Tracked as #983 with the shape a production-hook fix needs, so the gap is recorded rather than discovered later.How it is proved: a mutation —
abort_readmoved afterreading.await— fails ata closed subscription streams nothingwith the stored kind-1 event in the sink. The 200 ms injection cannot discriminate the new test (it never entershandlers::dispatch) and the job'sbuild.mdsays so rather than quoting a green as evidence.docs(relay): say what the CLOSE test owns, not that it owns a socket (#…