refactor: per-session locking in SessionManager (audit round 3, P7) - #29
Merged
Conversation
The sessions map was a single Arc<Mutex<HashMap<String, Session>>>, so any
operation on one session (key input, resize) blocked every other session and
client behind the same lock.
Change the map to Arc<Mutex<HashMap<String, Arc<Mutex<Session>>>>>. The new
SessionManager::session_arc(name) holds the map lock only to look up and clone
a session's Arc, then releases it; callers lock just that one session. All ~50
IPC call sites (pane/window/session/file dispatch, ws, handler) and the
internal manager methods were migrated to this pattern.
Lock order (deadlock prevention): map -> session -> workspace_state, and no two
of {session, workspace_state} are ever held simultaneously. list_workspaces,
rename_workspace, delete_workspace, to_snapshot, and poll_foreground_processes
were restructured to clone session Arcs under the map lock and then lock each
session separately, never overlapping with workspace_state.
pane_snapshot (sync, called from the WASM plugin host) uses try_lock on both
the map and each session, failing closed as before.
Tests: new concurrent_manager_ops_do_not_deadlock drives the locking paths from
8 tasks under a 10s timeout (PTY-free, runs in CI). cargo test (all crates) /
clippy --all-targets -D warnings / fmt --check green.
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
Coverage reportGenerated by |
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.
Implements finding P7 from the round 3 audit (docs/plans/audit-round3-2026h2.md).
Problem
The sessions map was a single
Arc<Mutex<HashMap<String, Session>>>. Every IPC operation locked the whole map, so any action on one session (key input, resize) blocked every other session and client behind the same lock.Change
Map is now
Arc<Mutex<HashMap<String, Arc<Mutex<Session>>>>>.SessionManager::session_arc(name)holds the map lock only to look up and clone a session'sArc, then releases it. Callers lock just that one session — the canonical by-name path.list_workspaces,rename_workspace,delete_workspace,to_snapshot, andpoll_foreground_processeswere restructured to clone sessionArcs under the map lock, then lock each session separately.pane_snapshot(sync, WASM plugin host) usestry_lockon both the map and each session, failing closed as before.Per-session window/pane lock granularity is intentionally left as a future item.
Tests
concurrent_manager_ops_do_not_deadlock: drives workspace + sessions-map + snapshot paths from 8 concurrent tasks under a 10s timeout — PTY-free, so it runs in CI (unlike the session-creating tests, which spawn PTYs and are#[ignore]).cargo test(all crates, 26 suites),cargo clippy --all-targets -- -D warnings,cargo fmt --checkall green.Notes
The migration bulk was mechanical (same transform at every call site); reviewed for the lock-order invariant above. Real throughput improvement can't be measured headless — the win is that per-session operations no longer serialize on one global lock.
Remaining deferred audit items: P3, P6 (profiling), P5, R5/A5 (PROTOCOL bump).