microsoft / microsoft/vscode-languageserver-node
Document lifecycle notifications parked across a crash-restart are delivered to the new server after the didOpen replay
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 1.8k
- Forks
- 404
- Avg merge
- 2d 8h
- Merged PRs (30d)
- 14
Description
vscode-languageclient (observed in 10.1.1) can deliver stale textDocument/didChange, textDocument/didClose, and textDocument/didOpen notifications to a freshly restarted server, out of order with the document-open replay the client performs on (re)connect. A server that validates document versions or open/closed state strictly will see an apparent protocol violation and, if it treats that as fatal, enter a crash loop.
Mechanism
- The server process crashes. The default
CloseAction.Restartpath runshandleConnectionClosed→cleanUp(ShutdownMode.Restart), which clears_syncedDocuments, and thenstart()on the same client instance (client.ts,handleConnectionClosed/cleanUp). - During the (multi-second) restart window, editor events keep firing.
DidChangeTextDocumentFeature.callbackcapturesdocument.versionat event time and callsclient.sendNotification(...). Because the client state is notStopped/Stopping,sendNotificationis not rejected — it parks atawait this.$start()and is later delivered over the next connection (client.ts,sendNotification:const connection = await this.$start();). The same applies todidClose/didOpen/didSavefired in that window. - On the new connection,
doInitializesets the state toRunningandinitializeFeaturesrunsDidOpenTextDocumentFeature.register, which replaysdidOpenfor every currently-open matching document with its current (post-window) version (textSynchronization.ts). - The parked stragglers from step 2 then flush against the new server:
- a
didChangecaptured at versionNarrives after the replayeddidOpenat versionM ≥ N→ the document version goes backwards; - a parked
didClosefor a document that was closed during the window → a close for a document the new server never saw opened (the replay only covers documents still open at replay time); - a parked
didOpen→ a second open for a document the replay already opened.
- a
Impact
The LSP spec says didChange versions increase after each change, so a server is entitled to treat a version going backwards (or a close/open for a document in the wrong state) as a protocol violation. In the Julia language server we assert on exactly that to catch real desyncs — and with this race, each straggler kills the freshly restarted server, so a single primary crash cascades into repeated crashes until the client's crash-loop breaker ("crashed 5 times in the last 3 minutes") gives up.
We observed this in the wild through the Julia VS Code extension's crash telemetry: right after a crash-restart, the new server received a replayed didOpen with version 3 followed by a parked didChange with version 2 ("LS version is 3, request version is 2"), and separately a didClose for a document that was never opened on the new connection. The reproduction rate tracks how often the server has a primary crash, which is what makes the window observable.
Reproduction sketch
- Server for some document type, restart-on-crash via the default error handler.
- Open a document, type (version reaches N).
- Kill the server process.
- While the client is restarting (state not
Running), keep typing (versions N+1, N+2 are captured and parked) and/or close the document. - When the restart completes, the server receives
didOpenat the current version followed by the parked notifications from step 4 captured at their older versions / for the now-closed document.
Proposed direction
Tag document-lifecycle notifications with the connection generation at the time they are created, and discard them at flush time (after await this.$start() resolves) if the connection has been swapped since. This is safe by construction: the restart replay re-establishes authoritative state for every open document — the replayed didOpen carries the document's complete current text — so a straggler from a previous connection is provably stale.
Related (all fixed) precedent for this class of ordering bug, none of which covers the crash-restart parking path: #1133 / #1176 (didChange/didOpen out of order with full sync during search-and-replace), #1228 (async param conversion breaking notification/request order), #1695 (delayOpenNotifications sending the wrong didOpen version).
We currently work around this in the Julia extension with middleware that defers lifecycle notifications fired while the client is not Running and drops the ones the replay has superseded (julia-vscode/julia-vscode#4205), but the durable fix belongs in the library.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in client.ts at handleConnectionClosed, cleanUp, sendNotification, and the restart path; then read textSynchronization.ts where didOpen replay is registered. Trace how lifecycle notifications wait for $start() and how the connection changes during restart. Done means notifications created for an earlier connection are discarded after a swap, while the new connection's didOpen replay remains authoritative.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- devtools
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100