Desktop session persistence used one deterministic temp path per session (<sid>.json.tmp) while releasing self.lock before disk I/O. Two threads persisting the same session could therefore write/replace the same temp file concurrently. A separate deletion race also allowed a late worker persist to recreate a session JSON after the session had been deleted from manager state.
Root cause
_persist_session() only held self.lock while building the data snapshot, then performed tmp.write_text() and os.replace() unlocked. _delete_session_file() also performed its filesystem operation without the same lock, and _persist_session() did not verify that the Session object was still registered before writing.
Fix
keep the existing RLock held across the session snapshot, temp write, and atomic replace
skip persistence when self.sessions.get(s.id) is not s, preventing stale workers from recreating deleted sessions
perform session-file deletion under the same lock
This preserves the existing per-session JSON format and atomic replace strategy.
Regression coverage
concurrent persists of the same session cannot overlap writes to <sid>.json.tmp
a late persist of a deleted Session object cannot recreate its JSON file
the test harness loads desktop_bridge.py only inside the unittest class lifecycle, uses an isolated temporary GA_ROOT, restores sys.argv and sys.modules, and does not leave import-time global state behind
Verification
TDD was performed on a separate validation branch:
Regression-only GitHub Actions run 31164526542 failed both targeted cases on current main
concurrent same-session temp writes overlapped and produced an actual FileNotFoundError from os.replace
a late _persist_session() recreated the file after delete_session()
The minimal locking/registration fix passed run 31164598932: compile, both deterministic concurrency regressions, and git diff --check
Review feedback identified test-global-state leakage. The harness was refactored without changing production code, and validation run 31165250013 passed compile, both regressions, and git diff --check
The contribution branch was then rebuilt directly from current main as one clean commit containing the production fix plus the isolated regression test
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
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.
Problem
Desktop session persistence used one deterministic temp path per session (<sid>.json.tmp) while releasing self.lock before disk I/O. Two threads persisting the same session could therefore write/replace the same temp file concurrently. A separate deletion race also allowed a late worker persist to recreate a session JSON after the session had been deleted from manager state.
Root cause
_persist_session() only held self.lock while building the data snapshot, then performed tmp.write_text() and os.replace() unlocked. _delete_session_file() also performed its filesystem operation without the same lock, and _persist_session() did not verify that the Session object was still registered before writing.
Fix
This preserves the existing per-session JSON format and atomic replace strategy.
Regression coverage
Verification
TDD was performed on a separate validation branch:
Scope