Repository navigation
fix(backend-pi): finish a duplicate-resume loser's teardown before its busy refusal (#3488) - #3501
Merged
Merged
Conversation
…s busy refusal (#3488) When two concurrent resumes of one non-resident session both open a worker, the one that returns second now decides busy or reuse (and claims a reused entry) synchronously, then awaits its own worker's shared inactive abort before re-raising the original outcome. The refusal settles only after the losing worker has exited, its turn temp is closed and its stdio release has been awaited. A refused cleanup is logged as a warning through the backend log and cannot replace the busy refusal; the reservation stays charged and the abort stays retryable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
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
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.
Closes #3488
What changed
When two concurrent resumes of one non-resident session both open a pi worker, the one that returns second still refuses with
BackendBusyError. That refusal now settles only after the losing worker's teardown has finished: the worker has exited, its turn temp has been closed exactly once, and its stdioreleased()has been awaited exactly once. Until now the loser only got a synchronousdispose(), which requests SIGKILL and returns. The turn-temp close ran detached on exit, its refusal was dropped, and nothing waited for the stdio release. While the refusal settled, the ledger still charged both workers' reservations. Now it charges only the winner's. If the loser's cleanup is refused, the caller still getsBackendBusyError. The refusal is logged throughbackend.logas a warning, the reservation stays charged, and the worker's sharedabort()can retry the close.Why this shape
This follows the issue's scope and the #2863 lifecycle ruling ("Turn completion includes worker teardown"). The losing resumed acquisition decides busy or reuse, and claims a reused entry, synchronously, before any await. That way a third acquisition can never see an unclaimed idle entry while the loser waits. The pool then awaits the losing worker's shared inactive
abort(), the same owner #2880'scancelAcquiredSessionuses, disposes the worker and re-raises the original outcome. Cleanup refusal is reported rather than substituted, following the backend's other preserved-cleanup warnings. The pool gets the existingBackendLogFnseam as an optional third argument, andcreatePiBackendpassesoptions.log. The unreachable new-session duplicate branch, publicdispose(), worker protocols, the busy and result shapes, and the five-second release bound are unchanged, as the issue's out-of-scope list requires. The decision record now notes #3488 under the #2880 paragraph. The investigation report's citations were re-anchored to the moved lines, and it now says the repair landed.Proof
Final head
2d80ed9409c5ee3d8f7ed3448cfbf6a60ed3b19fon basea9330064e56ffc82804e3025ce7c77d6127057a8(origin/main). Linux host withbwrap, keyless, private TMPDIR.Failing first (new
duplicate-acquisition.test.tsagainst unmodified maina272a790b; the source files this PR touches are the same on the current base):The test now runs twice: once with a confirmed loser cleanup and once with a refused one. At the moment the loser's
BackendBusyErrorsettles, it asserts the loser has exited, with 1 turn-temp close, 1released()call and 1 completedreleased(). With confirmed cleanup, the ledger charges only the winner's half and no warning is logged. With refused cleanup, both reservations stay charged and exactly one warning names the refusal; then a retry throughsession.abort()closes again (2 closes) and frees the loser's half. In both runs the winner is not signalled and is still running, it ends withsuccess, and every reservation is back at baseline. All of the test's earlier assertions are unchanged.Mutation receipts (on the final head; tree restored after each,
git statusclean):disposeSession(opened.session)instead of the awaited release)PI_DUPLICATE_RESUME_LOSER_EXITED_BEFORE_REFUSAL(expected true, received false)PI_DUPLICATE_RESUME_LOSER_REFUSES_BUSY(receivedundefined). The non-throwing loser then prompts, and the run's teardown waits until the timeout.PI_DUPLICATE_RESUME_REFUSED_LOSER_CLEANUP_REPORTED(expected length 1, received 0)PI_DUPLICATE_RESUME_LOSER_REFUSES_BUSY(received the cleanup refusal)Workspace tests that cannot run on this host.
bun run test packages/ui-backend-pigives 478 pass and 24 fail. The same 24 tests fail on unmodified main, checked by diffing the(fail)lists from a throwawayorigin/mainworktree. They need unprivileged user namespaces, which this host refuses (unshare: write failed /proc/self/uid_map: Operation not permitted). They are inhost-relay,native-host-auth-qualification,provider-history,provider-history-apify,session-prompt,stable-roster-runtime,startup-runtimeandtranscript-prefix, and are left to hosted CI. That covers the provider-history guard. This change adds nothing provider-facing: it only changes when a losing worker's refusal settles.{"version":1,"kind":"brain-check-pr-receipt","head":"2d80ed9409c5ee3d8f7ed3448cfbf6a60ed3b19f","base":"origin/main","mode":"focused","startedAt":"2026-11-03T15:17:01.811Z","finishedAt":"2026-11-03T15:18:40.963Z","checks":[{"name":"all lint and leakage gates","categories":["changeset"],"outcome":"passed","elapsedMs":8791.614308},{"name":"environment documentation","categories":["changeset"],"outcome":"passed","elapsedMs":94.48743199999808},{"name":"pending changeset package names","categories":["changeset"],"outcome":"passed","elapsedMs":1037.872212000002},{"name":"contribution changeset","categories":["changeset"],"outcome":"passed","elapsedMs":16.933732000001328},{"name":"affected fast invariants","categories":["verify","unit"],"outcome":"passed","elapsedMs":89192.416864}]}Checklist
docs/reflect the change, or nothing documented moved.docs/decisions/provider-history.md): the provider-history guard passes, or nothing on a turn's path changed. (No provider-facing content changed. The guard tests need user namespaces and are left to hosted CI.)Anything left open
ui-backend-pitests, has not been requested. This PR stays a draft.experiments/decision-citations.test.ts(release-time drift check from ci: check code-citation drift at release, not on every merge #3499) reports two drifts indocs/decisions/claude-code-runtime.mdcitingAGENTS.md:137-138. ci: check code-citation drift at release, not on every merge #3499'sAGENTS.mdedit caused them, not this PR, and they are left for the release check.🤖 Generated with Claude Code