Repository navigation
cmux-next: FolderChanges arms its watches before events() returns (fixes BackdropCatalogStoreTests hang) - #26320
Conversation
BackdropCatalogStoreTests.theCatalogIsBuiltOnceAndRebuiltOnlyWhenAFolderChanges hung for its 60 s limit under the parallel run: the watch was armed by a utility-priority detached task, which under load could start after the test added mine.png, so no event ever came. events() now arms before it returns, so a change after the call is always seen; FolderChangesTests drops the 100 ms head start it gave the loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-cc8bff1c3ddb4ec353c5
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Owned by an agent session · session |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
|
|
Found 6 test failures on Blacksmith runners: Failures
|
|
Bead |
|
Review (codex gpt-6-astra xhigh, next-review): SHIP at 90b418c No blocking findings. |
|
The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/40618800354 at 46f5779 failed: cmux-next checks (god files, concurrency, crash safety, l10n), cmux-next generated files. |
|
Shipped in:
|
…ered (#26517) * cmux-next: FolderChanges.events() returns once its watches are registered Post-landing review of #26320: arm() resumed each DispatchSource and returned, but resume() only schedules the kernel registration, and dispatch_source(3) says events before it completes are missed, so a write right after events() returned could still be lost. arm() now waits for every source's registration handler (events() is async), and the loop re-arms the same way before it yields. A new test holds the watches' queue suspended and checks events() returns only after it is let go, and that a write the moment it returns is seen. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-cc8bff1c3ddb4ec353c5 * test: start events() with Task.immediate so the readiness check is deterministic Review of the fix: the 200 ms sleep did not guarantee the task ran while the queue was held, so a regression could still pass. Task.immediate runs events() on the test's thread until its first suspension, which with the fix is the wait for registration and without it is never reached before it returns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-cc8bff1c3ddb4ec353c5 * FolderChanges: a caller cancelled during registration gets a finished stream Review of the fix: a cancelled caller that never iterates would leave the detached loop holding the watch and its descriptors. events() now cancels the watch and finishes the stream instead of starting the loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-cc8bff1c3ddb4ec353c5 --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Fixes a flake in the cmux-next swift test job (seen in job 122287803089, the full parallel CmuxNextAppTests run):
Cause:
FolderChanges.events()returned its stream and armed the folder watches (the DispatchSource file-system sources) later, inside a.utilitydetached task.BackdropCatalogStorecallsevents(), builds the catalog and bumpsbuilds. The test waits for that build and then addsmine.png. Under a loaded parallel run, the utility task could start only after the file was added. That change was never seen, so the wait for a second build never ended and the 60 s limit fired. The test already waits on a condition (an observed build counter) and not on a sleep. The missing event was the bug, so the time limit stays as it is.Change:
events()now arms the watches before it returns, and the loop task takes them over. So any change made afterevents()returns is buffered and seen.FolderChangesTestsdrops the 100 ms head start it gave the loop to arm, which this makes unnecessary.Proof (fleet suite gate): at head 90b418c, BackdropCatalogStoreTests passed 5/5 and FolderChangesTests passed 5/5 (no head-start sleep). The suite gate runs one suite per process, so the load that starved the utility task in the full parallel run does not happen there. The base red is only in CI's parallel run, and at base 68f7028 the gate passes 5/5 too. The fix rests on the ordering argument above, not on a reproduction.
No capture: nothing visible changes. This is the timing of a folder watch, and the wallpaper list looks the same before and after.
Review: Codex (gpt-6-astra, xhigh) gave SHIP at head 90b418c. It checked that the non-Sendable
FolderWatchmoves intoTask.detachedunder region isolation.Risk: low. The watch opens a few microseconds earlier, on the caller's thread rather than the loop's. Cancellation and re-arming are unchanged. The only other user is
BackdropCatalogStore.Agent session: agent session
🤖 Generated with Claude Code