fix(lsp): detach workspace/diagnostic/refresh from notification handlers to prevent deadlock - #25438
Open
hamodywe wants to merge 1 commit into
Open
fix(lsp): detach workspace/diagnostic/refresh from notification handlers to prevent deadlock#25438hamodywe wants to merge 1 commit into
hamodywe wants to merge 1 commit into
Conversation
…ers to prevent deadlock `workspace/diagnostic/refresh` is a server-to-client request, and a spec-compliant client may re-pull `textDocument/diagnostic` from this same server before replying. Awaiting that round-trip inside the notification handler that sent it pinned one of the transport's concurrency slots (tower-lsp-server defaults to 4) for the entire client round-trip. Four unanswered refreshes -- e.g. four rapid saves of a watched config file in pull-diagnostic mode -- pinned every slot: the server could no longer service the very diagnostic pulls the client was waiting on before replying. Circular wait, no timeout, no recovery until the process is killed; restarting the server re-wedges it within minutes under the same editor behavior. The refresh is advisory and nothing depends on its result beyond logging a warning, so spawn it detached instead: the handler returns its slot immediately and the server keeps answering requests while any number of refresh replies are outstanding. Applies to all three call sites (initialized, did_change_configuration, did_change_watched_files). The regression test drives the real wire transport at its default concurrency, holds 4 refresh replies open the way a busy client would, and asserts the server still answers -- against the previous code it times out exactly as reported. Fixes oxc-project#24955
hamodywe
force-pushed
the
fix/lsp-diagnostic-refresh-deadlock
branch
from
August 15, 2026 03:32
bf42082 to
c5b627e
Compare
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
2 participants
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.
What this PR does
Fixes the permanent language-server deadlock reported in #24955.
workspace/diagnostic/refreshis a server-to-client request, and a spec-compliant client may re-pulltextDocument/diagnosticfrom this same server before replying to it. Three notification handlers (initialized,did_change_configuration,did_change_watched_files) awaited that round-trip inside the handler itself, pinning one of the transport's concurrency slots (tower-lsp-server defaults to 4) for as long as the client took to reply. Four rapid watcher events — e.g. four saves of.oxlintrc.jsonor anytsconfig*.jsonin pull-diagnostic mode — pinned all four slots on handlers waiting for the client, while the client waited on diagnostic pulls the server could no longer service. Circular wait, no timeout, no recovery until the process is killed.The refresh is advisory — nothing consumes its result beyond logging a warning — so this PR detaches it: a new
Backend::spawn_diagnostic_refresh()clones the client and spawns the round-trip, returning the handler's slot immediately. The server keeps answering requests with any number of refresh replies outstanding. This is the issue's first suggested fix; raising the concurrency level (#25153) only widens the window rather than closing it, as the issue notes.How it's tested
test_outstanding_diagnostic_refreshes_do_not_wedge_the_serverruns in the crate's existing wire-levelTestServerharness — real LSP framing over duplex streams, the realServerat its real default concurrency of 4. It initializes in pull mode with dynamic watchers, fires four watched-file events, reads each resultingworkspace/diagnostic/refreshrequest without replying (what a busy client looks like), then sendsshutdownand requires a response within a 10s timeout.Verified both directions: against the previous code the test fails by timeout at exactly the reported wedge point; with the fix, the full crate suite passes (85/85).
cargo fmtandclippy --all-targetsare clean for the changed files. The only other change is adding tokio'stimefeature to dev-dependencies for the test's timeout.AI disclosure
Investigated, implemented, and verified with AI assistance (Claude Code), per the contributing guide's disclosure policy. I traced the root cause to the transport's concurrency accounting, confirmed the circular wait matches the reporter's reproduction, and validated the fix with the deadlock-reproducing regression test above before submission.