CHANGELOG.md

@@ -95,6 +95,17 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **LSP — `LspLifecycleManager.restart_server()` deadlock fix**: Refactored
+  `restart_server()` to use the same 3-phase lock pattern as `start_server()`,
+  releasing `self._lock` before all blocking I/O (Phase 1: lock+snapshot+remove
+  old entry; Phase 2: `stop()`/`start()`/`initialize()` without lock; Phase 3:
+  lock+insert new entry). Previously the lock was held across all blocking I/O
+  (up to 120 s), causing `health_check()`, `list_running()`, and `stop_server()`
+  callers to block for the full restart duration. The `ref_count` from the
+  original managed server is preserved across the restart. Three BDD scenarios
+  verify the fix using `threading.Barrier` synchronisation and
+  `acquire(blocking=False)` lock-state inspection. (#3165)
+
 - Fixed session leak in all `AutomationProfileRepository` public methods
   (`get_by_name()`, `list_all()`, `upsert()`, and `delete()`): added
   `finally: if self._auto_commit: session.close()` blocks matching the

docs/reference/lsp_lifecycle_restart.md

@@ -0,0 +1,73 @@
+# 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