forked from HAL9000/cleveragents-core
74 lines
2.4 KiB
Markdown
74 lines
2.4 KiB
Markdown
# 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
|