LspManager mutex is held across the post-edit diagnostics wait (up to 10s), serializing every other LSP operation

[iceteaSA]

lsp_notify_and_collect_diagnostics takes the LspManager mutex and keeps the guard alive across a blocking wait that can run for seconds.

// crates/aft/src/context.rs:5713
let Some(mut lsp) = self.lsp_manager.try_lock() else {
    return crate::lsp::manager::PostEditWaitOutcome::default();
};
…
// crates/aft/src/context.rs:5742 — guard `lsp` still alive
lsp.wait_for_post_edit_diagnostics(file_path, &config, &expected_versions, &pre_snapshot, timeout)

wait_for_post_edit_diagnostics loops on event_rx.recv_timeout(timeout) until its deadline. The timeout comes from wait_ms — default 3000 ms, capped at 10000 ms (context.rs:5959-5963).

For that whole window every other lsp_manager accessor blocks on the same mutex: lsp_notify_file_changed, lsp_clear_diagnostics_for_file, lsp_mark_diagnostics_stale_for_file, lsp_resync_changed_file_for_diagnostics, lsp_server_count.

This is the normal request path, not an edge case. lsp_post_write is called from every write/edit handler — write.rs:162, apply_patch.rs:318, edit_match.rs:1318, edit_symbol.rs:293, add_import.rs:630, batch.rs:164. Any edit with diagnostics: true (or an explicit wait_ms) takes the lock for the duration of the wait.

Your own comment at context.rs:5925-5927 notes that a tsserver re-analysis on a monorepo file routinely takes 2-5 s. That's the same window the whole LSP manager is serialized behind — and on a daemon serving several roots, an edit in one root blocks LSP bookkeeping for all of them.

Severity: P2. Needs concurrency to bite, but a multi-root daemon is exactly where it does, and the blocking window is set by an external process's analysis time rather than anything aft controls.

Fix direction: the wait doesn't need the manager — it needs the event channel. Snapshot the pre-edit state and expected versions under the lock, drop the guard, then wait on the channel. The try_lock already returns a default outcome on contention (context.rs:5713-5715), so the calling convention tolerates not getting the lock; shortening the hold makes that path rarer rather than changing semantics.

---

Provenance and limits. Found in a parallel read-only sweep; I verified the guard's lifetime across the wait firsthand (context.rs:5713 acquire → 5742 wait, same scope) before filing. What I have not done is measure contention — no profile showing a thread actually blocked, no wall-clock numbers. The claim is structural: the guard is alive across a bounded-but-long wait, and the other accessors take the same mutex. Whether that costs anything real depends on concurrent LSP traffic I haven't instrumented.

Line numbers against 28930e09.

[ualtinok]

Fixed in v0.50.1. The wait now runs off the manager mutex: pre-edit state and expected versions are snapshotted under the lock, a parked waiter is registered before the guard drops (no event-loss window between snapshot and wait start), and the recv loop runs lock-free. Your fix direction was right as filed — thanks for the precise blocked-accessor list; it became the concurrency regression's assertion set. A slow post-edit wait no longer blocks any other LSP operation, verified with a fake-server delay test that failed pre-fix.

[iceteaSA]

Verified the fix in-tree and re-deployed on it. One note for anyone who reads this thread later: the shipped fix is better than the one I proposed, and the difference is load-bearing.

I wrote "snapshot the pre-edit state and expected versions under the lock, drop the guard, then wait on the event channel." That has a gap — between dropping the guard and starting to wait, an event can arrive with no registered receiver and be lost.

What shipped registers the waiter before the drop:

// context.rs — comment is upstream's
// Register the wake receiver while the manager is still locked. Events
// that raced with registration remain on the raw receiver; events won by
// another drain path wake this waiter after that path updates the store.
let mut wait = lsp.start_post_edit_diagnostics_wait(file_path, &expected_versions, &pre_snapshot, timeout);
let mut complete = lsp.poll_post_edit_diagnostics_wait(&mut wait, None);
drop(lsp);

while !complete && !wait.deadline_reached() {
    let event = wait.next_event();
    let mut lsp = self.lsp_manager.lock();
    complete = lsp.poll_post_edit_diagnostics_wait(&mut wait, event);
}

Two things my version didn't have: the receiver exists before the lock is released, so nothing can slip through the seam; and the single poll before the drop short-circuits the case where diagnostics already landed, skipping the wait loop entirely.

Flagging it because the issue text and the implementation are both public, and the naive version is the one written down first.