fix(browser): bound the real-profile auth backup so a locked source falls through to raw copy - #96659
Open
liuhao1024 wants to merge 1 commit into
Open
liuhao1024 wants to merge 1 commit into
liuhao1024 wants to merge 1 commit into
Conversation
…alls through to raw copy _copy_auth_file() called sqlite3.Connection.backup() with no progress callback, and CPython retries SQLITE_BUSY forever in that loop — while a running Chrome holds Login Data / Web Data with exclusive SQLite locks on POSIX too, the first consented session hung until the tool timed out (~420 s) with no error, and the raw-copy fallback was unreachable (NousResearch#96646). The connect(timeout=5) the old comment relied on does not bound backup() retries. Reuse the deadline-bounded family helper hermes_cli.backup._safe_copy_db (same hazard already fixed there by NousResearch#82042/NousResearch#92495/NousResearch#84475): read-only connect with timeout=0, progress-callback deadline of _AUTH_BACKUP_TIMEOUT_SECONDS (10 s, matching the backup walker), and on failure return control to the existing raw-copy fallback, which needs no SQLite locks on POSIX. Fixes NousResearch#96646
13 tasks
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
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
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.
What does this PR do?
Bounds the online-backup call in
_copy_auth_file()so a locked auth DB can no longer hang the first consented real-profile session forever.With
browser.use_real_profile: trueand the user's Chrome running, Chrome holdsLogin Data/Web Datawith exclusive SQLite locks on POSIX too. The old code calledsource.backup(out)with no progress callback, and CPython'sbackup()retriesSQLITE_BUSYin a loop with no total timeout and no exception — so the copy never returned, theexceptnever fired, the raw-copy fallback was unreachable, andbrowser_execdied at the ~420 s tool timeout with no error (#96646). Thetimeout=5on the source connection the old comment relied on only bounds statement-level busy waits, notbackup()retries.The fix reuses
hermes_cli.backup._safe_copy_db— the deadline-bounded helper that already fixed this exact hazard class in the backup walker (#82042 / #92495 / #84475): read-only connect withtimeout=0.0, a progress-callback deadline (_AUTH_BACKUP_TIMEOUT_SECONDS, 10 s, matching the walker's default), fail-closedFalse. On failure, control returns to the existing raw-copy fallback, which needs no SQLite cross-process locks on POSIX, so the launch proceeds instead of hanging.Related Issue
Fixes #96646
Fixes #96661 (independent re-report of the same
_copy_auth_file()hang, filed with its own per-DB probe data; same root cause and same fix path)Type of Change
Changes Made
hermes_cli/browser_connect.py—_copy_auth_file()'s SQLite branch now callshermes_cli.backup._safe_copy_dbwith a 10 s budget instead of an unboundedsource.backup(out); added module constant_AUTH_BACKUP_TIMEOUT_SECONDS(monkeypatch-able for tests) and updated the docstring.tests/tools/test_browser_real_profile.py— regressiontest_copy_auth_file_bounded_when_locked_falls_back_to_raw: holds an exclusive SQLite lock on a realLogin DataDB (the way a running Chrome does), asserts_copy_auth_filereturns well within 5 s and succeeds via the raw-copy fallback with the copied DB readable.How to Test
python -m pytest tests/tools/test_browser_real_profile.py -q— should pass (74 passed, including the new regression).python -m pytest tests/hermes_cli/test_backup_all_profiles.py tests/hermes_cli/test_backup_path_errors.py tests/hermes_cli/test_backup_stability.py -q— should pass (16 passed; the shared helper's own suite).BEGIN EXCLUSIVEon the source, then_copy_auth_file) was still blocked after 90 s and had to be killed; on this branch the same call returns in ~0.6 s withTrueand a readable snapshot DB.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (targeted: real-profile suite 74 passed; backup family 16 passed)Documentation & Housekeeping
docs/, docstrings) — N/A (docstring updated in place)cli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A