# LSP Lifecycle Manager — `restart_server()` Lock Pattern ## Overview `LspLifecycleManager.restart_server()` uses a **3-phase lock pattern** to prevent deadlocks under concurrent access. This matches the pattern already used by `start_server()`. ## The Problem (pre-fix) Prior to PR #3165, `restart_server()` held `self._lock` across **all** blocking I/O operations: ``` acquire lock transport.stop() ← blocking I/O (up to 60s) transport.start() ← blocking I/O client.initialize() ← blocking I/O (up to 60s) release lock ``` Any concurrent caller of `health_check()`, `list_running()`, or `stop_server()` would block for up to 60 seconds waiting for the lock. ## The Fix — 3-Phase Lock Pattern ``` Phase 1 (lock held — short): Read current state, snapshot fields, remove old entry from _servers Phase 2 (lock RELEASED — blocking I/O): old_transport.stop() StdioTransport(...).start() client.initialize() Phase 3 (lock held — short): Insert new _ManagedServer into _servers, preserving ref_count ``` Removing the old entry in Phase 1 ensures concurrent callers see the server as absent during the restart window (consistent with the `start_server` contract). The `ref_count` from the original managed server is carried forward to the new one. ## Concurrency Guarantees | Concurrent operation | Behaviour during restart | |---------------------|--------------------------| | `health_check()` | Returns immediately (server absent in Phase 2) | | `list_running()` | Returns immediately (server absent in Phase 2) | | `stop_server()` | Returns immediately (server absent in Phase 2) | | `start_server()` | Proceeds normally (no lock contention in Phase 2) | ## BDD Coverage Three scenarios in `features/lsp_lifecycle_coverage.feature` verify the fix: 1. **`health_check is not blocked while restart_server is in progress`** — uses a `threading.Barrier` to synchronise two threads at the exact moment the lock is released in Phase 2. 2. **`restart_server does not hold the lock during client.initialize`** — inspects `_lock` state from within the mock `initialize()` side-effect using `acquire(blocking=False)`. 3. **`restart_server preserves the ref_count of the managed server`** — starts the server twice (ref_count=2), restarts it, asserts ref_count=2. ## References - PR #3165 — fix implementation - Issue #3026 — original bug report - `src/cleveragents/lsp/lifecycle.py` — implementation