Fix LSP crash loop: timing logs + panic crash reporting + handler isolation + reproduced panics - #1
Open
Feel-ix-343 wants to merge 3 commits into
Open
Conversation
The server would become unresponsive after a few minutes of editing a moderately large vault: code actions then go-to-definition time out, the server goes silent, and Zed's shutdown times out and force-resets the connection, restarting the cycle. Root cause is async-runtime starvation: - Every keystroke (didChange/didOpen) recomputes diagnostics over all open buffers synchronously, which is O(open_files x references x referenceables). As open buffers accumulate, each pass eventually takes longer than the gap between keystrokes, so passes pile up faster than they finish. - This CPU-bound work runs inline on the Tokio worker threads while holding the vault lock, and the forked tower-lsp caps request concurrency at 4. The server's bounded message queue fills, the stdin reader can no longer be scheduled, and shutdown / $/cancelRequest are never read -> silence. Fixes: - Debounce diagnostics: coalesce bursts of edits so only the latest change in a quiet window triggers a diagnostics pass, bounding the load. - Offload synchronous vault work with tokio::task::block_in_place so the runtime keeps servicing stdin and control messages under load. - Raise the request concurrency limit from the default 4 to 256. Co-Authored-By: Felix Zeller <felixazeller@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…nics A panic in any LSP handler does not exit the process (no panic=abort): it is caught at the task boundary, but it kills the request-dispatch loop, so the server stops reading stdin and goes silent. The editor then sees a hung server, times out shutdown, and force-restarts it — the reported crash loop — with no backtrace anywhere because the process never actually exits. Crash reporting (debug live): - install a global panic hook that logs every panic (message, location, full backtrace) to stderr (editors capture this in their LSP log) and to markdown-oxide-panic.log. - wrap the synchronous vault work in bind_vault/bind_vault_mut with catch_unwind so a panic in a query handler becomes a failed request instead of a silent server death. Fix the panics reproduced by driving the real server over LSP: - main.rs: empty didChange contentChanges -> remove(0) panic; use the last (full-document) change and ignore an empty list. - vault/mod.rs: file_stem().expect() on a stemless path -> handle gracefully. - commands.rs / link_completer.rs: date.format(dailynote).to_string() panics on an invalid format specifier; format via write! and skip on error. - tag_completer / link_completer / unindexed_block_completer: usize underflows (character-1, len()-1) -> checked_sub/saturating_sub. Co-Authored-By: Felix Zeller <felixazeller@gmail.com>
Time every user-facing request (definition, references, completion, codeAction, hover, documentSymbol, workspace/symbol, rename, codeLens, semanticTokens, inlayHint) via a shared timed() helper that logs '<method> took <ms>ms (ok|error)' at INFO, so slow requests are visible in the editor's LSP log. Also surface the indexing hot paths (update_vault, diagnostics, vault construction) at INFO with their durations. Folds the pre-existing ad-hoc completion/semantic-token timers into the same helper. Co-Authored-By: Felix Zeller <felixazeller@gmail.com>
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.
Summary
Three layers addressing the Zed crash loop and making future crashes/slowness debuggable live:
1. Per-request timing in the logs (INFO)
A shared
timed(label, fut)helper times every user-facing request and logs"<method> took <ms>ms (ok|error)"at INFO, so slow requests are visible in the editor's LSP log:Covers definition, references, completion, codeAction, hover, documentSymbol, workspace/symbol, rename, codeLens, semanticTokens, inlayHint, plus the indexing hot paths (
update_vault, diagnostics, vault construction). The pre-existing ad-hoc completion/semantic-token timers are folded into the same helper.2. Crash reporting (so panics are debuggable live)
A panic in an LSP handler does not exit the process (no
panic = "abort"): it's caught at the tokio task boundary, but it kills the request-dispatch path, so the server stops reading stdin and goes silent → editor times outshutdown→ force-restart (the crash loop), with no backtrace anywhere because the process never exits.install_panic_hook()(first inmain): logs every panic — message,file:line, full backtrace — to stderr (editors capture this) and tomarkdown-oxide-panic.log.catch_panic(context, f)wraps the synchronous vault work inbind_vault/bind_vault_mutso a panic becomes a failed request (InternalError) instead of a dead server, and is still fully reported by the hook.3. Panics reproduced over LSP and fixed
did_changecontentChanges→remove(0)MDFile::newfile:///) →file_stem().expect()file_stem().and_then(to_str).unwrap_or_default()commands.rs/link_completer.rs)dailynoteformat →date.format(fmt).to_string()panicswrite!(.., "{}", date.format(fmt)).ok()?tag_completer/link_completer/unindexed_block_completerchecked_sub/saturating_subValidation
window/logMessage) over a live LSP session.cargo test74/74; no new clippy/build warnings (the 3 insymbol.rs/commands.rsare pre-existing).Link to Devin session: https://app.devin.ai/sessions/5d37c33bc30944d58b1a376f829ce8d1
Requested by: @Feel-ix-343