Skip to content

simplex tests: a file is overdue counted from its forward, however long that took - #799

Merged
futurepaul merged 1 commit into
masterfrom
claude/simplex-cancel-test
Oct 31, 2026
Merged

futurepaul merged 1 commit into
masterfrom
claude/simplex-cancel-test

Conversation

@futurepaul

Copy link
Copy Markdown
Collaborator

Flake: simplex::run::tests::no_unsettled_cancel_is_let_go (images/bridge/src/simplex/run.rs:2744), "every cancel kept, none settled": 43 of 64 cancels left Cancelling. Seen on #779's images check (run 41161367564, job 124419427605), from master's code.

Cause (test): the test's clock (t) starts before its forwards and allows them 1 s (t += FILE_WAIT_MS + 1_000). But forward stamps each file with the real clock (Instant::now()) and writes the link's file each time. On a slow runner, a batch of 32 forwards took longer than that second:

  • the batch's later files weren't overdue at files_overdue(t), and stayed in files;
  • FILES_MAX (32) then turned away as many files of the next batch, which were never received and so never given up on;
  • so fewer than 64 cancels were kept: 64 − 21 = 43.

The product is right: a file is overdue a full FILE_WAIT_MS after its forward. Its twin, a_receive_asked_before_a_restart_keeps_its_place, relies on the same second, as do three single-file overdue steps.

Fix: the tests take an instant past the wait for every file forwarded so far, all_overdue(after) = after.max(Instant::now()) + FILE_WAIT_MS, wherever they relied on that second. It's no earlier than the clock they kept, so their retries and order are unchanged. Nothing waits longer: it's a computed instant, not a sleep.

Proof:

  • With each link-file write slowed by 40 ms (a proof-only edit, not committed; a batch takes about 1.3 s), the old tests fail: 57 of 64 cancels kept in this one, and its twin receives /freceive 66 it shouldn't. With the fix, both pass.
  • simplex::run::tests: 200 of 200 runs pass, plus 200 more as 8 at once (25 rounds), all passing. Clippy is clean with -D warnings.

🤖 Generated with Claude Code

…ng that took

The batch tests' clock started before their forwards and allowed 1 s for
them, but forward stamps each file with the real clock and writes the
link's file each time. On a slow runner a batch of 32 took longer, so
the later files were not yet overdue, stayed in `files`, and FILES_MAX
turned away as many of the next batch: 43 of 64 cancels kept (#779's
images check). The tests now take an instant past the wait for every
file forwarded so far (`all_overdue`), wherever they relied on that
second.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass at 4decfbd391a18fbee2b89a8c13e03131772ce04b.

The test-only helper removes the one-second assumption about forward/write duration: its deadline is at least FILE_WAIT_MS after every completed forward and advances the existing simulated clock. I traced the five changed call sites through file expiry, cancellation retries, and retained receive reservations; the assertions remain meaningful. One shared helper is proportionate to the fix, and I found no blocking regression.

Validation: CI is still running. Attempted cargo test -p fragment-bridge simplex::run::tests from images/, but this macOS host cannot compile the existing Linux-only libc::prctl/PR_SET_PDEATHSIG call (also present in the parent commit), so no local tests ran. The referenced external engineering-style file was unavailable on this machine.

@futurepaul
futurepaul added this pull request to the merge queue Oct 31, 2026
Merged via the queue into master with commit 7fc619c Oct 31, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants