fix(browser): real-profile auth mirror hangs forever on a locked destination - #105754
Closed
benbarclay wants to merge 1 commit into
Closed
benbarclay wants to merge 1 commit into
benbarclay wants to merge 1 commit into
Conversation
…ination A `browser_exec` call could park a thread in `sqlite3_sleep` permanently while mirroring Chrome's auth DBs, holding the agent's turn open. The turn never reaches its `finally`, so no `session.info running=false` settle is emitted and the Desktop composer latches busy — every later message queues and never sends. Captured live: one thread stuck 24+ minutes across two dumps, turn accepted at 15:03 with no `tui turn finished` 46 minutes later. Root cause is the DESTINATION, not the source. `Connection.backup()` retries a busy destination internally and ignores the connection's busy timeout, so `sqlite3.connect(dst, timeout=5)` cannot bound it. A destination left locked by an earlier hung mirror therefore blocks the next mirror forever — and because the tool-level 420s timeout abandons the thread without interrupting a C-level lock wait, the lock is never released and every subsequent launch re-hangs the same way. Self-perpetuating. Two changes: - Back up into a fresh `<dst>.new` and `os.replace()` it into place. No other process can hold a file we just created, so there is nothing to contend on, and the swap stays atomic. Measured against a live Chrome with a deliberately locked destination: 0.0006s vs an indefinite hang. - Drop the `mode=ro` (no `immutable=1`) source fallback. 8e74666 added `immutable=1` to fix exactly this hang but left `mode=ro` as a fallback, keeping the unbounded path one exception away; sqlite's busy timeout does not cover lock negotiation, so nothing bounds it. `immutable=1` is also the semantically correct mode — a committed snapshot of a file another process owns. The bounded plain-copy fallback is unchanged. Tests: three regressions, all mutation-checked (fail on base, pass here). The locked-destination test runs the copy on a worker with a join deadline so the unfixed behaviour fails fast instead of hanging the suite. 199 passing across the browser real-profile and CLI suites.
Contributor
૮ >ﻌ< ა ci reviewran on 47d942a — fix(browser): real-profile auth mirror hangs forever on a lo
|
alt-glitch
added
type/bug
Contributor
Related: #96659 (open, bounds the source-side backup and falls through to raw copy) targets the same real-profile auth-mirror hang family; #98249 (merged) covered the source |
teknium1
added a commit
that referenced
this pull request
Sep 8, 2026
Preserve Ben Barclay's diagnosis and replace the staging-file approach with SQLite-coordinated writes and a five-second backup callback deadline. A main-file replacement can replay an abandoned destination WAL; immutable source reads can miss committed source WAL. Neither raw copy nor replacement is safe when the destination is locked. Refuse unavailable auth databases without raw-copy fallback, retaining the existing close-browser-and-retry flow. Keep two invariant tests for lock refusal/recovery and source-versus-destination WAL contents. Convert existing text masquerading as database fixtures into real SQLite fixtures. Related: #105754 Related: #96659
Collaborator
|
Landed via #105763 at 13fb5e1. Thanks @benbarclay for the live diagnosis; your original commit and authorship are preserved. The salvage uses bounded SQLite backup in place rather than replacing the main file, avoiding abandoned destination WAL replay and preserving committed source WAL. It refuses locked/unreadable databases without raw-copy fallback. CI is green; the broader local browser sweep passed 686 tests. Linux snapshot/lock/recovery probes passed; native macOS/Desktop incident verification remains unclaimed. |
8 tasks
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
3 participants
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.
Symptom
A Desktop session finishes its work, then shows a permanent spinner; every message typed
afterwards piles into the composer queue and is never sent. Only a backend restart clears it.
Root cause
A
browser_exectool call hangs mirroring Chrome's auth DBs, holding the agent's turn open.Captured live with a SIGUSR2 thread dump:
The turn never reaches its
finally, so_emit_settled_session_infonever runs, nosession.info running=falsereaches the client, and the composer's queue drain (gated onbusy -> false) never fires.The destination is the culprit, not the source.
Connection.backup()retries a busydestination internally and ignores the connection's busy timeout, so
connect(dst, timeout=5)cannot bound it. A destination left locked by an earlier hung mirror blocks the next mirror
forever. The tool-level 420s timeout abandons the thread but cannot interrupt a C-level lock
wait, so the lock is never released and every subsequent launch re-hangs — self-perpetuating.
Evidence
sqlite3_sleeptui turn finished46 min laterOperationalError('database is locked')after 5.2smode=rosource URI vs live Chrometimeout=5never firedimmutable=1source URIChanges
<dst>.new, thenos.replace(). Nothing can hold a file we justcreated, so there is nothing to contend on; the swap stays atomic.
mode=rosource fallback. 8e74666 addedimmutable=1to fix this exact hangbut kept
mode=roas a fallback, leaving the unbounded path one exception away. sqlite's busytimeout does not cover lock negotiation, so nothing bounds it.
immutable=1is alsosemantically right: a committed snapshot of a file another process owns. The bounded
plain-copy fallback is unchanged.
Tests
Three regressions, all mutation-checked — verified failing on base, passing here:
test_copy_auth_file_never_opens_the_unbounded_ro_modetest_copy_auth_file_never_backs_up_into_the_live_destinationtest_copy_auth_file_cleans_up_temp_on_failureThe locked-destination test runs the copy on a worker with a join deadline, so the unfixed
behaviour fails fast rather than hanging the suite (confirmed: the naive version hung a full
pytest run past 300s).
199 passedacrosstests/tools/test_browser_real_profile.pyandtests/tools/test_browser_use_cli.py.Scope note
This fixes the
browser_exechang. It does not add a general backstop for "a tool wedges inC code and the turn never settles" — the tool timeout abandons such threads without unwinding
them. That looks worth addressing separately; happy to follow up if you agree.