bsp: a task finishing between two state reads could be admitted twice - #635
Merged
Conversation
…reak bleep fmt Fallout from an OOM incident report against M10, where the evidence trail went cold twice: - `bleep config compile-server stop-all` deleted a socket dir holding 10GB of heap dumps without a word. It still deletes — that is its job — but now logs each dir with its size, at warn level when it was ≥1GB, so gigabytes of diagnostics no longer vanish silently. - The failure path printed `BSP server log: <path>` for a file that did not exist (server never started, or the dir was already cleaned), sending the reader to a dead end. Both call sites now say `(missing — …)` when the file is not there. Separately, `bleep fmt` failed repo-wide: scalafmt cannot parse a lambda whose body is an indented try/catch followed by `, arg` in the same call (ServerRunInterruptTest.scala, from #629). Verified against scalafmt 3.11.5 under dialects scala3/scala36/scala3future — none parse it, so the fix is binding the Runnable to a val first. Bumped scalafmt 3.11.1 → 3.11.5 while at it; zero reformat churn. Co-Authored-By: Claude Fable 5 <[email protected]>
The scheduler loop snapshotted dagRef before runningRef, while a completing task writes the other way around: finished into dagRef (end of executeTask), then removed from runningRef (its guarantee). A task finishing between the two reads was therefore visible in neither snapshot — not finished in the stale dag, not running in the fresh set — so ready.filterNot(running) admitted it a second time and the task ran twice. Reading runningRef first closes the window: since finished-in-dag happens-before removed-from-running, a started task is always visible in at least one of the two snapshots. Same swap applied to the post-admission re-read for the same invariant. Caught on CI by LinkDagIntegrationTest "executor: emits link events" — two LinkStarted(myapp-js) events 1ms apart. The test's instant IO.pure handlers make the window hittable under load; in production this is any task kind running twice. Co-Authored-By: Claude Fable 5 <[email protected]>
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.
Root cause of the
LinkDagIntegrationTest / "executor: emits link events"failure on master's CI run for #634 (twoLinkStarted(myapp-js)events 1 ms apart).The scheduler loop snapshotted
dagRefbeforerunningRef, while a completing task writes the other way around: finished intodagRef(end ofexecuteTask), then removed fromrunningRef(itsguarantee). A task finishing between the two reads was visible in neither snapshot — not finished in the stale dag, not running in the fresh set — soready.filterNot(running)admitted it a second time and the task ran twice. Not link-specific: any task kind (compiles included) could double-run under the same timing.Reading
runningReffirst closes the window: finished-in-dag happens-before removed-from-running, so a started task is always visible in at least one of the two snapshots. The post-admission re-read gets the same swap for the same invariant.Verified:
LinkDagIntegrationTest25/25 green locally,LinkExecutorIntegrationTestgreen,bleep fmtclean.🤖 Generated with Claude Code