Skip to content

Stopping a client with a pending full-sync didChange logs "Delivering pending changes failed: TypeError: this.task is not a function" #1916

Open

Description

Versions: vscode-languageclient 10.1.2, 11.0.0, and main at e4ddcca, built from source. The code dates from 8.1.0 (#1141). In 8.1.0 and 9.0.1, I checked only outside VS Code that the client's own code path fails the same way. I tested with VS Code 1.143.1 on Linux x64, vscode-languageserver 11.0.0, and Node 24.

When a client using TextDocumentSyncKind.Full stops while an edit is still waiting in the 250 ms pending-change debounce, the stopped client's timer still fires. About 500 ms after the edit, the client logs:

[error] Delivering pending changes failed
TypeError: this.task is not a function
	at .../node_modules/vscode-languageclient/lib/common/utils/async.js:38:37

Reproduction

package.json:

{
  "name": "pending-change-repro",
  "publisher": "repro",
  "version": "0.0.1",
  "engines": { "vscode": "^1.91.0" },
  "main": "extension.js",
  "activationEvents": ["onLanguage:plaintext"],
  "contributes": { "commands": [{ "command": "repro.editAndRestart", "title": "Repro: Edit and Restart" }] },
  "dependencies": { "vscode-languageclient": "11.0.0", "vscode-languageserver": "11.0.0" }
}

server.js:

const { createConnection, TextDocumentSyncKind } = require('vscode-languageserver/node');
const connection = createConnection();
connection.onInitialize(() => ({ capabilities: { textDocumentSync: TextDocumentSyncKind.Full } }));
connection.onDidOpenTextDocument(({ textDocument: { version, text } }) =>
  connection.console.info(`server ${process.pid} didOpen v${version} ${JSON.stringify(text)}`));
connection.onDidChangeTextDocument(({ textDocument: { version }, contentChanges }) =>
  connection.console.info(`server ${process.pid} didChange v${version} ${JSON.stringify(contentChanges[0].text)}`));
connection.listen();

extension.js:

const path = require('path');
const vscode = require('vscode');
const { LanguageClient, TransportKind } = require('vscode-languageclient/node');

let channel;
let client;

function createClient() {
  const server = { module: path.join(__dirname, 'server.js'), transport: TransportKind.ipc };
  return new LanguageClient('repro', 'Repro LSP', { run: server, debug: server }, {
    documentSelector: [{ scheme: 'file', language: 'plaintext' }],
    // One channel for every client, so the stopped client's log stays visible.
    outputChannel: channel,
  });
}

exports.activate = async (context) => {
  channel = vscode.window.createOutputChannel('Repro LSP', { log: true });
  context.subscriptions.push(channel, vscode.commands.registerCommand('repro.editAndRestart', async () => {
    await vscode.window.activeTextEditor.edit((edit) => edit.insert(new vscode.Position(0, 0), 'unsaved edit\n'));
    // Restart before the 250 ms full-sync debounce delivers the edit.
    await client.stop();
    client = createClient();
    await client.start();
  }));
  client = createClient();
  await client.start();
};

exports.deactivate = () => client?.stop();
  1. Run npm install, then start the extension, for example with code --extensionDevelopmentPath="$PWD" <folder>.
  2. Open a .txt file and wait for the client to start.
  3. Run Repro: Edit and Restart, then open the Repro LSP output channel.

Actual output. It failed in 10 of 10 runs each with 10.1.2 and 11.0.0:

18:21:36.162 [info] server 33225 didOpen v1 "hello\n"
18:21:37.221 [info] Server process exited successfully
18:21:37.274 [info] server 34095 didOpen v2 "unsaved edit\nhello\n"
18:21:37.711 [error] Delivering pending changes failed
TypeError: this.task is not a function
	at .../node_modules/vscode-languageclient/lib/common/utils/async.js:38:37

Expected: no error. After stop(), the client shouldn't keep a pending-change timer. The new client's didOpen already carries the unsaved text at version 2, so nothing needs to be delivered.

Other ways to reach it

I ran each case 10 times in VS Code against 10.1.2, 11.0.0 and main. All three versions gave the same results:

Case Error logged
stop() and a new client, outputChannel shared (above) 10/10
stop() and a new client, each client owns its channel 0/10: the same failure happens, but the log is dropped, because the stopped client has disposed its channel (shouldLogToOutputChannel())
client.restart(), server answers initialize immediately 0/10
client.restart(), server takes 500 ms to answer initialize 10/10
Server process killed; the default error handler restarts it, and the server answers immediately 0/10
Same, with the server taking 500 ms to answer initialize 10/10

Restart and crash recovery reuse the same client. They fail when the 250 ms timer fires before the new connection is running. Slow-starting servers hit this after crashes, so the error shows up in crash logs and gets mistaken for the cause of the crash. For example: tamasfe/taplo#787, gp-pereira/refactorex#18, cwtools/cwtools-vscode#159. An issue search for both strings finds 27 issues and pull requests in other repositories since 2023, most of them crash reports quoting this log. In every run, the next connection's didOpen carried the exact unsaved text and version. The edit isn't lost; the error is spurious.

Cause

Links are to main at e4ddcca:

  1. cleanUp() cancels _fileEventDelayer but not _pendingChangeDelayer, so the timer outlives stop(), and it outlives cleanUp(ShutdownMode.Restart) after a crash.
  2. When it fires, triggerPendingChangeDelivery() finds no active connection and calls itself synchronously, inside the running task, to retry.
  3. Delayer.trigger()'s continuation runs this.task!() and only then sets this.task = undefined. That erases the task the re-entrant trigger() just stored, while its new timer stays armed.
  4. 250 ms later that timer calls undefined. The rejection reaches the .catch on line 1772, which logs it without a notification.

The Delayer part reproduces on its own:

const d = new Delayer(0);
let retry;
await d.trigger(() => { retry = d.trigger(() => 'second'); });
await retry; // rejects: TypeError: this.task is not a function

forceDelivery() has the same ordering problem. If its task triggers again, forceDelivery() clears onSuccess, and the new timer throws an uncaught TypeError: this.onSuccess is not a function. Nothing calls forceDelivery() today.

Suggested fix

diff --git a/client/src/common/client.ts b/client/src/common/client.ts
index 16678792..5537eb67 100644
--- a/client/src/common/client.ts
+++ b/client/src/common/client.ts
@@ -1679,6 +1679,9 @@ export abstract class BaseLanguageClient implements FeatureClient<Middleware, La
 		// purge outstanding file events.
 		this._fileEvents = [];
 		this._fileEventDelayer.cancel();
+		// Drop a pending full text document sync. Clearing the features below
+		// drops the pending changes, and a new connection reopens the documents.
+		this._pendingChangeDelayer.cancel();
 
 		const disposables = this._listeners.splice(0, this._listeners.length);
 		for (const disposable of disposables) {
diff --git a/client/src/common/utils/async.ts b/client/src/common/utils/async.ts
index 99a7033f..281303fb 100644
--- a/client/src/common/utils/async.ts
+++ b/client/src/common/utils/async.ts
@@ -36,9 +36,10 @@ export class Delayer<T> {
 			}).then(() => {
 				this.completionPromise = undefined;
 				this.onSuccess = undefined;
-				const result = this.task!();
+				// Detach the task before running it: the task may trigger again.
+				const task = this.task!;
 				this.task = undefined;
-				return result;
+				return task();
 			});
 		}
 
@@ -57,11 +58,11 @@ export class Delayer<T> {
 			return undefined;
 		}
 		this.cancelTimeout();
-		const result: T = this.task!();
+		const task = this.task!;
 		this.completionPromise = undefined;
 		this.onSuccess = undefined;
 		this.task = undefined;
-		return result;
+		return task();
 	}
 
 	public isTriggered(): boolean {

The cleanUp() change is the essential part. On its own, it removed the error in every case above: 0 of 60 runs, with the unsaved text still reopened in 60 of 60. The Delayer change alone also silences the error. But then a stopped client retries every 250 ms forever, because it never gets a connection again. I still recommend the Delayer change alongside the cleanUp() one, because it makes re-entrant tasks safe. With both changes: 0 of 60 runs logged the error, and 60 of 60 reopened the text.

With both changes, npm --prefix client-node-tests test passes on VS Code Insiders 1.144.0: 225 tests, including the test below. On current main, that test fails with TypeError: this.task is not a function, and the other 224 pass.

A regression test for client-node-tests/src/converter.test.ts:

suite('Delayer', () => {

	test('A task can trigger the delayer again', async () => {
		const delayer = new async.Delayer<void>(0);
		let retriggered = false;
		let retry: Promise<void> | undefined;
		await delayer.trigger(async () => {
			retry = delayer.trigger(async () => { retriggered = true; });
		});
		await retry;
		ok(retriggered);
	});
});

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions