Repository navigation
A request can be sent after shutdown when stop() starts while sendRequest awaits pending document synchronization #1921
Description
Versions: vscode-languageclient 10.1.2 and 11.0.0 (with vscode-languageserver-protocol 3.18.4 and 3.18.5, and vscode-jsonrpc 9.0.3 and 10.0.0), and main at e4ddcca. VS Code 1.142.0 on Linux. The server uses full document sync (TextDocumentSyncKind.Full).
What happens. A request can be written to the server after the client has sent shutdown and before it sends exit:
initialize, initialized, textDocument/didOpen, shutdown, textDocument/documentSymbol, exit
shutdown is a request, so the connection stays open until its response and exit; the request is written on it in between. The request's promise is rejected or the request cancelled when the client stops, and the server's answer is dropped with the connection, so nothing is lost, and the server answers it as LSP requires: with InvalidRequest (-32600). But the client's entry check rejects requests once the client is Stopping or Stopped, so this is a request that should never have been written.
In VS Code, the trigger is ordinary UI work: after a crashed server is recovered, VS Code's breadcrumbs request textDocument/documentSymbol as soon as the client registers the provider. That request waits behind the recovered client's pending didOpen notifications. If Virgil: Restart Language Server (or a window close) stops the client during that wait, the request follows shutdown. Our Extension Host checks assert that only exit follows shutdown, so this made one of them fail once (the run received shutdown, textDocument/documentSymbol, $/cancelRequest, exit).
Cause. BaseLanguageClient.sendRequest (client/src/common/client.ts:914–929 on main; lib/common/client.js:581–592 in 10.1.2):
- rejects the request if the client is
StartFailed,Stopping, orStopped, but only on entry; - awaits
$start(); - awaits
this._didOpenTextDocumentFeature.sendPendingOpenNotifications(); - under full sync, awaits
this.sendPendingFullTextDocumentChanges(connection); - only then calls
connection.sendRequest.
stop() sets $state = ClientState.Stopping (client.ts:1638), cleans up, and sends shutdown and then exit (client.ts:1642–1646). If it begins during any of the awaits in step 2–4, nothing re-checks the state, and the request is written after shutdown.
sendNotification has the same missing re-check: it checks the state on entry, then awaits $start() and, under full sync, the pending full-text changes (and, for most notifications, the pending opens), then writes (client.ts:1020–1057; client.js:671–715 in 10.1.2). A didOpen can follow shutdown for that reason too, which our tests already allow (they filter didOpen from the shutdown sequence).
Minimal reproduction (library only, no VS Code). Install vscode-languageclient in an empty directory, then run node repro.cjs .. The script stubs the vscode module, runs a LanguageClient against a fake server over in-memory streams, holds a pending didOpen behind a paused pipe, and calls stop() while a sendRequest waits:
const path = require('node:path');
const Module = require('node:module');
const { PassThrough } = require('node:stream');
const base = path.resolve(process.argv[2] ?? '.');
const requireFrom = (id) => require(require.resolve(id, { paths: [base] }));
// A stand-in for the vscode module: only what the client touches here.
const disposable = () => ({ dispose() {} });
const inert = () => disposable();
class Uri {
constructor(scheme, p) { this.scheme = scheme; this.path = p; }
static file(p) { return new Uri('file', p); }
static parse(s) { const m = /^([^:]+):\/\/(.*)$/.exec(s); return new Uri(m[1], m[2]); }
toString() { return `${this.scheme}://${this.path}`; }
}
class EventEmitter {
constructor() { this.listeners = new Set(); this.event = (l) => { this.listeners.add(l); return disposable(); }; }
fire(v) { for (const l of [...this.listeners]) l(v); }
dispose() { this.listeners.clear(); }
}
const channel = {
logLevel: 3, onDidChangeLogLevel: inert, appendLine() {}, info() {}, warn() {}, error() {},
trace() {}, debug() {}, show() {}, dispose() {},
};
const vscode = new Proxy({
Uri, EventEmitter, Disposable: { from: inert },
Position: class { constructor(line, character) { this.line = line; this.character = character; } },
Range: class { constructor(start, end) { this.start = start; this.end = end; } },
CancellationError: class extends Error {},
CancellationTokenSource: class { constructor() { this.token = { isCancellationRequested: false }; } cancel() {} dispose() {} },
LogLevel: { Off: 0, Trace: 1, Debug: 2, Info: 3, Warning: 4, Error: 5 },
workspace: {
textDocuments: [], notebookDocuments: [], workspaceFolders: undefined,
getConfiguration: () => ({ get: (_n, d) => d, has: () => false, inspect() {}, update: async () => {} }),
createFileSystemWatcher: () => ({ onDidCreate: inert, onDidChange: inert, onDidDelete: inert, dispose() {} }),
onDidOpenTextDocument: inert, onDidChangeTextDocument: inert, onDidCloseTextDocument: inert,
onDidSaveTextDocument: inert, onWillSaveTextDocument: inert, onDidChangeConfiguration: inert,
onDidChangeWorkspaceFolders: inert,
},
window: {
createOutputChannel: () => channel, visibleTextEditors: [], activeTextEditor: undefined,
tabGroups: { all: [], onDidChangeTabs: inert, onDidChangeTabGroups: inert },
onDidChangeVisibleTextEditors: inert, onDidChangeActiveTextEditor: inert,
showInformationMessage: async () => {}, showErrorMessage: async () => {}, showWarningMessage: async () => {},
withProgress: async (_o, task) => task({ report() {} }, {}),
},
languages: {
match: () => 1,
createDiagnosticCollection: () => ({ set() {}, delete() {}, clear() {}, dispose() {}, forEach() {} }),
},
commands: { registerCommand: inert, executeCommand: async () => {} },
env: { language: 'en' },
version: '1.142.0',
}, { get: (target, name) => (name in target ? target[name] : class {}) });
const originalLoad = Module._load;
Module._load = function (request, ...rest) {
return request === 'vscode' ? vscode : originalLoad.call(this, request, ...rest);
};
// The client and a fake server over in-memory streams.
const { StreamMessageReader, StreamMessageWriter } = requireFrom('vscode-jsonrpc/node');
const { LanguageClient } = require(path.join(base, 'vscode-languageclient/lib/common/client.js'));
const toServer = new PassThrough({ highWaterMark: 1 });
const toClient = new PassThrough();
const received = [];
let shutdownId;
const reader = new StreamMessageReader(toServer);
const writer = new StreamMessageWriter(toClient);
reader.listen((m) => {
if (m.method === 'initialize') {
received.push('initialize');
writer.write({ jsonrpc: '2.0', id: m.id, result: { capabilities: { textDocumentSync: { openClose: true, change: 1 } } } });
} else if (m.method === 'initialized') {
received.push('initialized');
} else if (m.method === 'shutdown') {
received.push('shutdown');
shutdownId = m.id; // answered after 500 ms, or when a request arrives
setTimeout(() => shutdownId !== undefined && (writer.write({ jsonrpc: '2.0', id: shutdownId, result: null }), shutdownId = undefined), 500);
} else if (m.method === 'exit') {
received.push('exit');
} else if (m.method) {
received.push(m.method);
writer.write({ jsonrpc: '2.0', id: m.id, result: null });
if (shutdownId !== undefined) { writer.write({ jsonrpc: '2.0', id: shutdownId, result: null }); shutdownId = undefined; }
}
});
const client = new LanguageClient('repro', 'Repro', () => ({
reader: new StreamMessageReader(toClient), writer: new StreamMessageWriter(toServer),
}), { documentSelector: [{ scheme: 'file', language: 'virgil' }] });
(async () => {
await client.start();
const document = { uri: Uri.file('/tmp/main.v3'), languageId: 'virgil', version: 1, getText: () => '' };
client._didOpenTextDocumentFeature._pendingOpenNotifications.set(document.uri.toString(), document);
toServer.pause(); // the server is busy: the didOpen write stays in flight
const request = client.sendRequest({ method: 'textDocument/documentSymbol' },
{ textDocument: { uri: document.uri.toString() } }).catch((e) => `rejected: ${e.message}`);
await new Promise((r) => setTimeout(r, 50));
const stopping = client.stop(); // queues shutdown behind the didOpen write
await new Promise((r) => setTimeout(r, 50));
toServer.resume();
await new Promise((r) => setTimeout(r, 700));
await stopping;
console.log(received.join(', '));
console.log(await request);
})();Output with 10.1.2:
initialize, initialized, textDocument/didOpen, shutdown, textDocument/documentSymbol, exit
null
Output with 11.0.0 is identical. The result was the same in 10 of 10 runs on each version. With an experiment that adds the same state check again after the awaits in sendRequest, the output is initialize, initialized, textDocument/didOpen, shutdown, exit and the request rejects with Client is not running (5 of 5 runs), which shows the reproduction exercises the missing check.
Suggested fix. Re-check StartFailed, Stopping, and Stopped after the awaits in sendRequest (and in sendNotification, for the same reason), before the middleware and connection.sendRequest. The request already ends with ConnectionInactive once the connection is disposed; rejecting it at the point where the state is known is what the entry check intends. Alternatively, the connection could refuse requests once shutdown has been sent.
Workaround. Our Extension Host tests turn VS Code's breadcrumbs off in the recovery checks, which removes the automatic document-symbol request without weakening the checks' shutdown assertion. Users see nothing: the server's InvalidRequest reply is dropped with the connection, and the request's promise is rejected as cancelled.
Found while testing virgil-language-server's VS Code client.
Activity