fix: Servers terminating even with 'detached: true'

client-node-tests/src/integration.test.ts

@@ -10,7 +10,9 @@ import * as vscode from 'vscode';
 import * as lsclient from 'vscode-languageclient/node';
 import * as proto from 'vscode-languageserver-protocol';
 import { MemoryFileSystemProvider } from './memoryFileSystemProvider';
-import { vsdiag, DiagnosticProviderMiddleware } from 'vscode-languageclient';
+import { vsdiag, DiagnosticProviderMiddleware, LanguageClient } from 'vscode-languageclient';
+import { IsDetachedRequest } from './servers/types';
+import { afterEach } from 'mocha';
 
 namespace GotNotifiedRequest {
 	export const method: 'testing/gotNotified' = 'testing/gotNotified';
@@ -1961,6 +1963,60 @@ class CrashClient extends lsclient.LanguageClient {
 }
 
 suite('Server tests', () => {
+	suite('detached', async function () {
+		const detachedServerModule = path.join(__dirname, 'servers', 'detachedServer.js');
+
+		let client: LanguageClient | undefined;
+
+		afterEach(async function () {
+			await client?.stop();
+		});
+
+		test('servers are NOT detached by default', async () => {
+			const serverOptions: lsclient.ServerOptions = {
+				module: detachedServerModule,
+				transport: lsclient.TransportKind.ipc,
+			};
+			const client = new lsclient.LanguageClient('test svr', serverOptions, {});
+			const res = await client.sendRequest(IsDetachedRequest);
+			assert.deepStrictEqual(res, { detached: false });
+		});
+
+		[lsclient.TransportKind.stdio, lsclient.TransportKind.ipc, lsclient.TransportKind.pipe].forEach((transport) => {
+			test(`server detects it is detached using Node ServerOptions when transport: ${transport}`, async () => {
+				const serverOptions: lsclient.ServerOptions = {
+					module: detachedServerModule,
+					transport,
+					options: {
+						detached: true,
+						detachedTimeout: 1000
+					}
+				};
+				const client = new lsclient.LanguageClient('test svr', serverOptions, {});
+				const res = await client.sendRequest(IsDetachedRequest);
+				assert.deepStrictEqual(res, { detached: true, timeout: 1000 });
+			});
+		});
+
+		[lsclient.TransportKind.stdio, lsclient.TransportKind.pipe].forEach((transport) => {
+			test(`server detects it is detached using Executable ServerOptions when transport: ${transport}`, async () => {
+				const serverOptions: lsclient.ServerOptions = {
+					command: 'node', // making assumption this exists in the PATH of the test environment
+					args: [detachedServerModule],
+					transport,
+					options: {
+						detached: true,
+						detachedTimeout: 1001
+					}
+				};
+				const client = new lsclient.LanguageClient('test svr', serverOptions, {});
+				const res = await client.sendRequest(IsDetachedRequest);
+				assert.deepStrictEqual(res, { detached: true, timeout: 1001 });
+			});
+		});
+
+	});
+
 	test('Stop fails if server crashes after shutdown request', async () => {
 		const serverOptions: lsclient.ServerOptions = {
 			module: path.join(__dirname, './servers/crashOnShutdownServer.js'),

client-node-tests/src/servers/detachedServer.ts

@@ -0,0 +1,30 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+/**
+ * Server that allows the client to request detached status of the server
+ */
+
+import { createConnection, ProposedFeatures } from 'vscode-languageserver/node';
+import { IsDetachedRequest } from './types';
+import { parseCliOpts } from 'vscode-languageserver/utils';
+
+const connection = createConnection(ProposedFeatures.all);
+
+connection.onRequest(IsDetachedRequest, () => {
+	const args = parseCliOpts(process.argv);
+	const detached = Object.keys(args).includes('detached');
+	const timeout = args['detached'];
+	return { detached, timeout };
+});
+
+// Initialize the language server connection
+connection.onInitialize(() => {
+	return {
+		capabilities: {}
+	};
+});
+
+connection.listen();

client-node-tests/src/servers/types.ts

@@ -0,0 +1,8 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { RequestType0 } from 'vscode-languageserver';
+
+export const IsDetachedRequest = new RequestType0<{detached: boolean; timeout: number }, void>('isDetached');

client/src/node/main.ts

@@ -57,10 +57,18 @@ namespace Transport {
 	}
 }
 
-export interface ExecutableOptions {
+export interface DetachedOptions {
+	detached?: boolean;
+	/**
+	 * Maximum milliseconds to keep the server alive after the parent/client terminates.
+	 * If undefined it will never terminate.
+	 */
+	detachedTimeout?: number;
+}
+
+export interface ExecutableOptions extends DetachedOptions {
 	cwd?: string;
 	env?: any;
-	detached?: boolean;
 	shell?: boolean;
 }
 
@@ -77,7 +85,7 @@ namespace Executable {
 	}
 }
 
-export interface ForkOptions {
+export interface ForkOptions extends DetachedOptions {
 	cwd?: string;
 	env?: any;
 	encoding?: string;
@@ -130,6 +138,7 @@ export class LanguageClient extends BaseLanguageClient {
 	private readonly _serverOptions: ServerOptions;
 	private readonly _forceDebug: boolean;
 	private _serverProcess: ChildProcess | undefined;
+	/** Use {@link _setDetached} to set the value unless necessary */
 	private _isDetached: boolean | undefined;
 	private _isInDebugMode: boolean;
 
@@ -285,20 +294,22 @@ export class LanguageClient extends BaseLanguageClient {
 		if (Is.func(server)) {
 			return server().then((result) => {
 				if (MessageTransports.is(result)) {
-					this._isDetached = !!result.detached;
+					this._setDetached(!!result.detached);
 					return result;
 				} else if (StreamInfo.is(result)) {
-					this._isDetached = !!result.detached;
+					this._setDetached(!!result.detached);
 					return { reader: new StreamMessageReader(result.reader), writer: new StreamMessageWriter(result.writer) };
 				} else {
 					let cp: ChildProcess;
+					let isDetached;
 					if (ChildProcessInfo.is(result)) {
 						cp = result.process;
-						this._isDetached = result.detached;
+						isDetached = result.detached;
 					} else {
 						cp = result;
-						this._isDetached = false;
+						isDetached = false;
 					}
+					this._setDetached(isDetached, cp);
 					cp.stderr!.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 					return { reader: new StreamMessageReader(cp.stdout!), writer: new StreamMessageWriter(cp.stdin!) };
 				}
@@ -348,13 +359,20 @@ export class LanguageClient extends BaseLanguageClient {
 					} else if (Transport.isSocket(transport)) {
 						args.push(`--socket=${transport.port}`);
 					}
+					if (node.options?.detached) {
+						args.push('--detached');
+						if (node.options.detachedTimeout) {
+							args.push(node.options.detachedTimeout.toString());
+						}
+					}
 					args.push(`--clientProcessId=${process.pid.toString()}`);
 					if (transport === TransportKind.ipc || transport === TransportKind.stdio) {
 						const serverProcess = cp.spawn(runtime, args, execOptions);
 						if (!serverProcess || !serverProcess.pid) {
 							return handleChildProcessStartError(serverProcess, `Launching server using runtime ${runtime} failed.`);
 						}
 						this._serverProcess = serverProcess;
+						this._setDetached(!!node.options?.detached, serverProcess);
 						serverProcess.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 						if (transport === TransportKind.ipc) {
 							serverProcess.stdout.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
@@ -369,6 +387,7 @@ export class LanguageClient extends BaseLanguageClient {
 								return handleChildProcessStartError(process, `Launching server using runtime ${runtime} failed.`);
 							}
 							this._serverProcess = process;
+							this._setDetached(!!node.options?.detached, process);
 							process.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 							process.stdout.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 							return transport.onConnected().then((protocol) => {
@@ -382,6 +401,7 @@ export class LanguageClient extends BaseLanguageClient {
 								return handleChildProcessStartError(process, `Launching server using runtime ${runtime} failed.`);
 							}
 							this._serverProcess = process;
+							this._setDetached(!!node.options?.detached, process);
 							process.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 							process.stdout.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 							return transport.onConnected().then((protocol) => {
@@ -404,6 +424,12 @@ export class LanguageClient extends BaseLanguageClient {
 							args.push(`--socket=${transport.port}`);
 						}
 						args.push(`--clientProcessId=${process.pid.toString()}`);
+						if (node.options?.detached) {
+							args.push('--detached');
+							if (node.options.detachedTimeout) {
+								args.push(node.options.detachedTimeout.toString());
+							}
+						}
 						const options: cp.ForkOptions = node.options ?? Object.create(null);
 						options.env = getEnvironment(options.env, true);
 						options.execArgv = options.execArgv || [];
@@ -413,6 +439,7 @@ export class LanguageClient extends BaseLanguageClient {
 							const sp = cp.fork(node.module, args || [], options);
 							assertStdio(sp);
 							this._serverProcess = sp;
+							this._setDetached(!!options.detached, sp);
 							sp.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 							if (transport === TransportKind.ipc) {
 								sp.stdout.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
@@ -425,6 +452,7 @@ export class LanguageClient extends BaseLanguageClient {
 								const sp = cp.fork(node.module, args || [], options);
 								assertStdio(sp);
 								this._serverProcess = sp;
+								this._setDetached(!!options.detached, sp);
 								sp.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 								sp.stdout.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 								transport.onConnected().then((protocol) => {
@@ -436,6 +464,7 @@ export class LanguageClient extends BaseLanguageClient {
 								const sp = cp.fork(node.module, args || [], options);
 								assertStdio(sp);
 								this._serverProcess = sp;
+								this._setDetached(!!options.detached, sp);
 								sp.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 								sp.stdout.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 								transport.onConnected().then((protocol) => {
@@ -460,6 +489,12 @@ export class LanguageClient extends BaseLanguageClient {
 				} else if (transport === TransportKind.ipc) {
 					throw new Error(`Transport kind ipc is not support for command executable`);
 				}
+				if (command.options?.detached) {
+					args.push('--detached');
+					if (command.options.detachedTimeout) {
+						args.push(command.options.detachedTimeout.toString());
+					}
+				}
 				const options = Object.assign({}, command.options);
 				options.cwd = options.cwd || serverWorkingDir;
 				if (transport === undefined || transport === TransportKind.stdio) {
@@ -469,7 +504,7 @@ export class LanguageClient extends BaseLanguageClient {
 					}
 					serverProcess.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 					this._serverProcess = serverProcess;
-					this._isDetached = !!options.detached;
+					this._setDetached(!!options.detached, serverProcess);
 					return Promise.resolve({ reader: new StreamMessageReader(serverProcess.stdout), writer: new StreamMessageWriter(serverProcess.stdin) });
 				} else if (transport === TransportKind.pipe) {
 					return createClientPipeTransport(pipeName!).then((transport) => {
@@ -478,7 +513,7 @@ export class LanguageClient extends BaseLanguageClient {
 							return handleChildProcessStartError(serverProcess, `Launching server using command ${command.command} failed.`);
 						}
 						this._serverProcess = serverProcess;
-						this._isDetached = !!options.detached;
+						this._setDetached(!!options.detached, serverProcess);
 						serverProcess.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 						serverProcess.stdout.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 						return transport.onConnected().then((protocol) => {
@@ -492,7 +527,7 @@ export class LanguageClient extends BaseLanguageClient {
 							return handleChildProcessStartError(serverProcess, `Launching server using command ${command.command} failed.`);
 						}
 						this._serverProcess = serverProcess;
-						this._isDetached = !!options.detached;
+						this._setDetached(!!options.detached, serverProcess);
 						serverProcess.stderr.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 						serverProcess.stdout.on('data', data => this.outputChannel.append(Is.string(data) ? data : data.toString(encoding)));
 						return transport.onConnected().then((protocol) => {
@@ -568,6 +603,21 @@ export class LanguageClient extends BaseLanguageClient {
 		return Promise.resolve(undefined);
 	}
 
+	/**
+	 * Setter that should be used for {@link _isDetached}. It performs additional
+	 * actions based on the value set for detached.
+	 */
+	private _setDetached(isDetached: boolean, serverProcess?: cp.ChildProcess) {
+		if (!isDetached) {
+			this._isDetached = false;
+			return;
+		}
+
+		this._isDetached = true;
+		// Ensures the parent can exit even if the child (server) is still running
+		serverProcess?.unref();
+	}
+
 }
 
 export class SettingMonitor {

server/package.json

@@ -24,6 +24,10 @@
 		"./browser": {
 			"types": "./lib/browser/main.d.ts",
 			"browser": "./lib/browser/main.js"
+		},
+		"./utils": {
+			"types": "./lib/common/utils/main.d.ts",
+			"default": "./lib/common/utils/main.js"
 		}
 	},
 	"bin": {

server/src/common/utils/main.ts

@@ -0,0 +1,6 @@
+/* --------------------------------------------------------------------------------------------
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License. See License.txt in the project root for license information.
+ * ------------------------------------------------------------------------------------------ */
+
+export * from './process';