From d5b42a2d6c7a2e7d2c9a2e36105704553c599a26 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:25:40 -0700 Subject: [PATCH 1/5] client: reject start() if server process exits before connecting on pipe/socket transport Motivation: When using TransportKind.pipe or TransportKind.socket, the client creates a pipe/socket server and waits for the spawned server process to connect to it via transport.onConnected(). That promise (created in createClientPipeTransport / createClientSocketTransport in jsonrpc/src/node/main.ts) is only ever resolved from inside the connection callback - it has no reject path. If the server process exits before connecting (for example because it fails to start, or exits with a nonzero code), the promise never settles and LanguageClient#start() hangs forever instead of failing. Approach: Add a small waitForConnection() helper in client/src/node/main.ts that races transport.onConnected() against the spawned child process's 'exit' event, rejecting with a descriptive error if the process exits first. Use it at all six pipe/socket call sites in createMessageTransports (module+runtime, forked module, and Executable/command branches), in place of the bare transport.onConnected().then(...) calls. Validation: - npm run compile (tsc -b) is clean in jsonrpc, protocol, types, textDocument, server, client and client-node-tests after `npm run symlink`. - npm run lint is clean in client and client-node-tests (only a pre-existing, unrelated eslint-plugin deprecation warning). - Added two integration tests in client-node-tests/src/integration.test.ts ('Socket transport' and new 'Pipe transport' suites) using a new minimal server script (servers/exitBeforeConnectServer.ts) that exits immediately without ever connecting, mirroring the reporter's repro. Ran via `npm test` in client-node-tests (real VS Code Insiders + Electron extension host, @vscode/test-electron): before the fix the new socket test times out after 5s (client.start() never settles, reproducing the reported hang exactly); after the fix both new tests pass, and the full existing suite (207 tests) passes with no regressions. - Additionally reproduced the root cause directly with a standalone script using the repo's own built jsonrpc createClientSocketTransport plus a real spawned process that exits non-zero without connecting, confirming transport.onConnected() never settles without the fix and rejects quickly with the fix in place. This only changes what happens when the server process exits before the pipe/socket connection is established - normal startup and shutdown behavior (including the process 'exit' logging in the finally() block) is unaffected, since the added listener is a no-op once the connection promise has already settled. Report: https://github.com/microsoft/vscode-languageserver-node/issues/1586 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code) --- client-node-tests/src/integration.test.ts | 31 ++++++++++++++++ .../src/servers/exitBeforeConnectServer.ts | 6 ++++ client/src/node/main.ts | 35 +++++++++---------- 3 files changed, 53 insertions(+), 19 deletions(-) create mode 100644 client-node-tests/src/servers/exitBeforeConnectServer.ts diff --git a/client-node-tests/src/integration.test.ts b/client-node-tests/src/integration.test.ts index 766933ad0..98f7e9bee 100644 --- a/client-node-tests/src/integration.test.ts +++ b/client-node-tests/src/integration.test.ts @@ -309,6 +309,37 @@ suite('Socket transport', () => { // would time out. Reaching this line proves the actual bound port was handed over. await client.stop(); }); + + test('start() rejects if the server process exits before connecting', async () => { + const serverOptions: lsclient.ServerOptions = { + module: path.join(__dirname, './servers/exitBeforeConnectServer.js'), + transport: { kind: lsclient.TransportKind.socket, port: 0 }, + }; + const clientOptions: lsclient.LanguageClientOptions = {}; + const client = new lsclient.LanguageClient('socket exit before connect', 'Socket Transport (exits before connecting)', serverOptions, clientOptions); + // Before the fix start() would hang forever since the client only waited on + // the socket connection and never observed the server process exiting first. + await assert.rejects(async () => { + await client.start(); + }, /Server process exited before the connection could be established/); + }).timeout(5000); +}); + +suite('Pipe transport', () => { + + test('start() rejects if the server process exits before connecting', async () => { + const serverOptions: lsclient.ServerOptions = { + module: path.join(__dirname, './servers/exitBeforeConnectServer.js'), + transport: lsclient.TransportKind.pipe, + }; + const clientOptions: lsclient.LanguageClientOptions = {}; + const client = new lsclient.LanguageClient('pipe exit before connect', 'Pipe Transport (exits before connecting)', serverOptions, clientOptions); + // Same underlying bug as the socket transport case: start() would hang forever + // instead of surfacing the server process exiting before it connected the pipe. + await assert.rejects(async () => { + await client.start(); + }, /Server process exited before the connection could be established/); + }).timeout(5000); }); suite('Client integration', () => { diff --git a/client-node-tests/src/servers/exitBeforeConnectServer.ts b/client-node-tests/src/servers/exitBeforeConnectServer.ts new file mode 100644 index 000000000..2ecadac57 --- /dev/null +++ b/client-node-tests/src/servers/exitBeforeConnectServer.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. + *--------------------------------------------------------------------------------------------*/ + +process.exit(42); diff --git a/client/src/node/main.ts b/client/src/node/main.ts index 3c64f7e87..a3d5ef9b6 100644 --- a/client/src/node/main.ts +++ b/client/src/node/main.ts @@ -16,7 +16,7 @@ import * as Is from '../common/utils/is'; import { BaseLanguageClient, LanguageClientOptions as BaseLanguageClientOptions, MessageTransports, ShutdownMode } from '../common/client'; import { terminate } from './processes'; -import { StreamMessageReader, StreamMessageWriter, IPCMessageReader, IPCMessageWriter, createClientPipeTransport, generateRandomPipeName, createClientSocketTransport, InitializeParams} from 'vscode-languageserver-protocol/node'; +import { StreamMessageReader, StreamMessageWriter, IPCMessageReader, IPCMessageWriter, createClientPipeTransport, generateRandomPipeName, createClientSocketTransport, InitializeParams, MessageReader, MessageWriter} from 'vscode-languageserver-protocol/node'; // Import SemVer functions individually to avoid circular dependencies in SemVer import semverParse = require('semver/functions/parse'); @@ -420,9 +420,7 @@ export class LanguageClient extends BaseLanguageClient { this._serverProcess = process; pipeStderr(process.stderr, this.outputChannel); pipeStdout(process.stdout, this.outputChannel); - return transport.onConnected().then((protocol) => { - return { reader: protocol[0], writer: protocol[1] }; - }); + return waitForConnection(process, transport.onConnected()); }); } else if (Transport.isSocket(transport)) { return createClientSocketTransport(transport.port).then((transport) => { @@ -435,9 +433,7 @@ export class LanguageClient extends BaseLanguageClient { this._serverProcess = process; pipeStderr(process.stderr, this.outputChannel); pipeStdout(process.stdout, this.outputChannel); - return transport.onConnected().then((protocol) => { - return { reader: protocol[0], writer: protocol[1] }; - }); + return waitForConnection(process, transport.onConnected()); }); } } else { @@ -476,9 +472,7 @@ export class LanguageClient extends BaseLanguageClient { this._serverProcess = sp; pipeStderr(sp.stderr, this.outputChannel); pipeStdout(sp.stdout, this.outputChannel); - transport.onConnected().then((protocol) => { - resolve({ reader: protocol[0], writer: protocol[1] }); - }, reject); + waitForConnection(sp, transport.onConnected()).then(resolve, reject); }, reject); } else if (Transport.isSocket(transport)) { createClientSocketTransport(transport.port).then((transport) => { @@ -488,9 +482,7 @@ export class LanguageClient extends BaseLanguageClient { this._serverProcess = sp; pipeStderr(sp.stderr, this.outputChannel); pipeStdout(sp.stdout, this.outputChannel); - transport.onConnected().then((protocol) => { - resolve({ reader: protocol[0], writer: protocol[1] }); - }, reject); + waitForConnection(sp, transport.onConnected()).then(resolve, reject); }, reject); } }); @@ -530,9 +522,7 @@ export class LanguageClient extends BaseLanguageClient { this._isDetached = !!options.detached; pipeStderr(serverProcess.stderr, this.outputChannel); pipeStdout(serverProcess.stdout, this.outputChannel); - return transport.onConnected().then((protocol) => { - return { reader: protocol[0], writer: protocol[1] }; - }); + return waitForConnection(serverProcess, transport.onConnected()); }); } else if (Transport.isSocket(transport)) { return createClientSocketTransport(transport.port).then((transport) => { @@ -546,9 +536,7 @@ export class LanguageClient extends BaseLanguageClient { this._isDetached = !!options.detached; pipeStderr(serverProcess.stderr, this.outputChannel); pipeStdout(serverProcess.stdout, this.outputChannel); - return transport.onConnected().then((protocol) => { - return { reader: protocol[0], writer: protocol[1] }; - }); + return waitForConnection(serverProcess, transport.onConnected()); }); } } @@ -652,6 +640,15 @@ export class SettingMonitor { } } +function waitForConnection(childProcess: ChildProcess, connected: Promise<[MessageReader, MessageWriter]>): Promise { + return new Promise((resolve, reject) => { + childProcess.once('exit', (code, signal) => { + reject(new Error(`Server process exited before the connection could be established (code: ${code}, signal: ${signal}).`)); + }); + connected.then((protocol) => resolve({ reader: protocol[0], writer: protocol[1] }), reject); + }); +} + function handleChildProcessStartError(process: ChildProcess, message: string) { if (process === null) { return Promise.reject(message); From 3776a91446cd6f9b9ca4044aea11b82f6f97b809 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:54:50 -0700 Subject: [PATCH 2/5] Release pipe/socket listener when the server exits before connecting Address review feedback: add dispose() to the jsonrpc client PipeTransport/SocketTransport, call it from waitForConnection when the child exits before connecting and on the spawn-failure early returns, and add jsonrpc tests that the port/pipe path is released. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- client/src/node/main.ts | 26 ++++++++++--------- jsonrpc/src/node/main.ts | 28 ++++++++++++--------- jsonrpc/src/node/test/connection.test.ts | 32 +++++++++--------------- 3 files changed, 42 insertions(+), 44 deletions(-) diff --git a/client/src/node/main.ts b/client/src/node/main.ts index a3d5ef9b6..0365904e6 100644 --- a/client/src/node/main.ts +++ b/client/src/node/main.ts @@ -414,26 +414,26 @@ export class LanguageClient extends BaseLanguageClient { return createClientPipeTransport(pipeName!).then((transport) => { const process = cp.spawn(runtime, args, execOptions); if (!process || !process.pid) { - transport.close(); + transport.dispose(); return handleChildProcessStartError(process, `Launching server using runtime ${runtime} failed.`); } this._serverProcess = process; pipeStderr(process.stderr, this.outputChannel); pipeStdout(process.stdout, this.outputChannel); - return waitForConnection(process, transport.onConnected()); + return waitForConnection(process, transport); }); } else if (Transport.isSocket(transport)) { return createClientSocketTransport(transport.port).then((transport) => { args.push(`--socket=${transport.port()}`); const process = cp.spawn(runtime, args, execOptions); if (!process || !process.pid) { - transport.close(); + transport.dispose(); return handleChildProcessStartError(process, `Launching server using runtime ${runtime} failed.`); } this._serverProcess = process; pipeStderr(process.stderr, this.outputChannel); pipeStdout(process.stdout, this.outputChannel); - return waitForConnection(process, transport.onConnected()); + return waitForConnection(process, transport); }); } } else { @@ -472,7 +472,7 @@ export class LanguageClient extends BaseLanguageClient { this._serverProcess = sp; pipeStderr(sp.stderr, this.outputChannel); pipeStdout(sp.stdout, this.outputChannel); - waitForConnection(sp, transport.onConnected()).then(resolve, reject); + waitForConnection(sp, transport).then(resolve, reject); }, reject); } else if (Transport.isSocket(transport)) { createClientSocketTransport(transport.port).then((transport) => { @@ -482,7 +482,7 @@ export class LanguageClient extends BaseLanguageClient { this._serverProcess = sp; pipeStderr(sp.stderr, this.outputChannel); pipeStdout(sp.stdout, this.outputChannel); - waitForConnection(sp, transport.onConnected()).then(resolve, reject); + waitForConnection(sp, transport).then(resolve, reject); }, reject); } }); @@ -515,28 +515,28 @@ export class LanguageClient extends BaseLanguageClient { return createClientPipeTransport(pipeName!).then((transport) => { const serverProcess = cp.spawn(command.command, args, options); if (!serverProcess || !serverProcess.pid) { - transport.close(); + transport.dispose(); return handleChildProcessStartError(serverProcess, `Launching server using command ${command.command} failed.`); } this._serverProcess = serverProcess; this._isDetached = !!options.detached; pipeStderr(serverProcess.stderr, this.outputChannel); pipeStdout(serverProcess.stdout, this.outputChannel); - return waitForConnection(serverProcess, transport.onConnected()); + return waitForConnection(serverProcess, transport); }); } else if (Transport.isSocket(transport)) { return createClientSocketTransport(transport.port).then((transport) => { args.push(`--socket=${transport.port()}`); const serverProcess = cp.spawn(command.command, args, options); if (!serverProcess || !serverProcess.pid) { - transport.close(); + transport.dispose(); return handleChildProcessStartError(serverProcess, `Launching server using command ${command.command} failed.`); } this._serverProcess = serverProcess; this._isDetached = !!options.detached; pipeStderr(serverProcess.stderr, this.outputChannel); pipeStdout(serverProcess.stdout, this.outputChannel); - return waitForConnection(serverProcess, transport.onConnected()); + return waitForConnection(serverProcess, transport); }); } } @@ -640,12 +640,14 @@ export class SettingMonitor { } } -function waitForConnection(childProcess: ChildProcess, connected: Promise<[MessageReader, MessageWriter]>): Promise { +function waitForConnection(childProcess: ChildProcess, transport: { onConnected(): Promise<[MessageReader, MessageWriter]>; dispose(): void }): Promise { return new Promise((resolve, reject) => { childProcess.once('exit', (code, signal) => { + // The server never connected, so stop listening to release the pipe / port. + transport.dispose(); reject(new Error(`Server process exited before the connection could be established (code: ${code}, signal: ${signal}).`)); }); - connected.then((protocol) => resolve({ reader: protocol[0], writer: protocol[1] }), reject); + transport.onConnected().then((protocol) => resolve({ reader: protocol[0], writer: protocol[1] }), reject); }); } diff --git a/jsonrpc/src/node/main.ts b/jsonrpc/src/node/main.ts index fbc64a1b5..6db327273 100644 --- a/jsonrpc/src/node/main.ts +++ b/jsonrpc/src/node/main.ts @@ -196,12 +196,10 @@ export function generateRandomPipeName(): string { export interface PipeTransport { onConnected(): Promise<[MessageReader, MessageWriter]>; /** - * Closes the underlying server if a client hasn't connected yet. Callers - * should invoke this if they abandon the transport before `onConnected` - * resolves (for example because the server process failed to start) to - * avoid leaking the listening pipe. + * Stops listening for a connection. Use this to release the pipe if the + * other side never connects (e.g. the server process exited early). */ - close(): void; + dispose(): void; } export function createClientPipeTransport(pipeName: string, encoding: RAL.MessageBufferEncoding = 'utf-8'): Promise { @@ -222,7 +220,11 @@ export function createClientPipeTransport(pipeName: string, encoding: RAL.Messag server.removeListener('error', reject); resolve({ onConnected: () => { return connected; }, - close: () => { server.close(); } + dispose: () => { + if (server.listening) { + server.close(); + } + } }); }); }); @@ -240,12 +242,10 @@ export interface SocketTransport { port(): number; onConnected(): Promise<[MessageReader, MessageWriter]>; /** - * Closes the underlying server if a client hasn't connected yet. Callers - * should invoke this if they abandon the transport before `onConnected` - * resolves (for example because the server process failed to start) to - * avoid leaking the listening socket. + * Stops listening for a connection. Use this to release the port if the + * other side never connects (e.g. the server process exited early). */ - close(): void; + dispose(): void; } export function createClientSocketTransport(port: number, encoding: RAL.MessageBufferEncoding = 'utf-8'): Promise { @@ -273,7 +273,11 @@ export function createClientSocketTransport(port: number, encoding: RAL.MessageB resolve({ port: () => boundPort, onConnected: () => { return connected; }, - close: () => { server.close(); } + dispose: () => { + if (server.listening) { + server.close(); + } + } }); }); }); diff --git a/jsonrpc/src/node/test/connection.test.ts b/jsonrpc/src/node/test/connection.test.ts index ee4c514d2..6dbdc2ceb 100644 --- a/jsonrpc/src/node/test/connection.test.ts +++ b/jsonrpc/src/node/test/connection.test.ts @@ -104,38 +104,30 @@ suite('Connection', () => { assert.ok(bound > 0, `expected a positive bound port, got ${bound}`); }); - test('createClientSocketTransport close closes the listening socket before a client connects', async () => { + test('createClientSocketTransport releases the port on dispose', async () => { const net = await import('net'); const transport = await hostConnection.createClientSocketTransport(0); const bound = transport.port(); - - transport.close(); - + transport.dispose(); + const probe = net.createServer(); await new Promise((resolve, reject) => { - const client = net.connect(bound, '127.0.0.1'); - client.on('connect', () => { - client.destroy(); - reject(new Error('expected the connection to be refused after close')); - }); - client.on('error', () => resolve()); + probe.on('error', reject); + probe.listen(bound, '127.0.0.1', () => resolve()); }); + await new Promise(resolve => probe.close(() => resolve())); }); - test('createClientPipeTransport close closes the listening pipe before a client connects', async () => { + test('createClientPipeTransport releases the pipe on dispose', async () => { const net = await import('net'); const pipeName = hostConnection.generateRandomPipeName(); const transport = await hostConnection.createClientPipeTransport(pipeName); - - transport.close(); - + transport.dispose(); + const probe = net.createServer(); await new Promise((resolve, reject) => { - const client = net.createConnection(pipeName); - client.on('connect', () => { - client.destroy(); - reject(new Error('expected the connection to be refused after close')); - }); - client.on('error', () => resolve()); + probe.on('error', reject); + probe.listen(pipeName, () => resolve()); }); + await new Promise(resolve => probe.close(() => resolve())); }); test('Test Duplex Stream Connection', (done) => { From bdfb867f0a74c4efaf79676215c8714685dc9a8b Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Wed, 30 Sep 2026 02:42:17 -0700 Subject: [PATCH 3/5] client: settle start() when the server process fails to spawn Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- client/src/node/main.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/src/node/main.ts b/client/src/node/main.ts index 0365904e6..31727fbcf 100644 --- a/client/src/node/main.ts +++ b/client/src/node/main.ts @@ -647,6 +647,11 @@ function waitForConnection(childProcess: ChildProcess, transport: { onConnected( transport.dispose(); reject(new Error(`Server process exited before the connection could be established (code: ${code}, signal: ${signal}).`)); }); + // A failed spawn (e.g. invalid execPath) emits 'error' and never 'exit'. + childProcess.once('error', (error) => { + transport.dispose(); + reject(error); + }); transport.onConnected().then((protocol) => resolve({ reader: protocol[0], writer: protocol[1] }), reject); }); } From 0a2015b4a1ac75a94679a62d19fe7230b2219942 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Wed, 30 Sep 2026 23:15:32 -0700 Subject: [PATCH 4/5] client: keep the transports' close() API and detach exit/error listeners Renaming PipeTransport/SocketTransport close() to dispose() broke the API that main already exports (and that is re-exported through the protocol and client node entry points). Revert jsonrpc to main's close() and its tests, and call close() from waitForConnection instead. waitForConnection now also removes its 'exit' and 'error' listeners from the child process as soon as the connection is established or fails, so a server exiting normally later no longer runs them. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-opus-5-5 (via Claude Code) --- client/src/node/main.ts | 36 ++++++++++++++++-------- jsonrpc/src/node/main.ts | 28 ++++++++---------- jsonrpc/src/node/test/connection.test.ts | 32 +++++++++++++-------- 3 files changed, 57 insertions(+), 39 deletions(-) diff --git a/client/src/node/main.ts b/client/src/node/main.ts index 31727fbcf..a9606e76b 100644 --- a/client/src/node/main.ts +++ b/client/src/node/main.ts @@ -414,7 +414,7 @@ export class LanguageClient extends BaseLanguageClient { return createClientPipeTransport(pipeName!).then((transport) => { const process = cp.spawn(runtime, args, execOptions); if (!process || !process.pid) { - transport.dispose(); + transport.close(); return handleChildProcessStartError(process, `Launching server using runtime ${runtime} failed.`); } this._serverProcess = process; @@ -427,7 +427,7 @@ export class LanguageClient extends BaseLanguageClient { args.push(`--socket=${transport.port()}`); const process = cp.spawn(runtime, args, execOptions); if (!process || !process.pid) { - transport.dispose(); + transport.close(); return handleChildProcessStartError(process, `Launching server using runtime ${runtime} failed.`); } this._serverProcess = process; @@ -515,7 +515,7 @@ export class LanguageClient extends BaseLanguageClient { return createClientPipeTransport(pipeName!).then((transport) => { const serverProcess = cp.spawn(command.command, args, options); if (!serverProcess || !serverProcess.pid) { - transport.dispose(); + transport.close(); return handleChildProcessStartError(serverProcess, `Launching server using command ${command.command} failed.`); } this._serverProcess = serverProcess; @@ -529,7 +529,7 @@ export class LanguageClient extends BaseLanguageClient { args.push(`--socket=${transport.port()}`); const serverProcess = cp.spawn(command.command, args, options); if (!serverProcess || !serverProcess.pid) { - transport.dispose(); + transport.close(); return handleChildProcessStartError(serverProcess, `Launching server using command ${command.command} failed.`); } this._serverProcess = serverProcess; @@ -640,19 +640,33 @@ export class SettingMonitor { } } -function waitForConnection(childProcess: ChildProcess, transport: { onConnected(): Promise<[MessageReader, MessageWriter]>; dispose(): void }): Promise { +function waitForConnection(childProcess: ChildProcess, transport: { onConnected(): Promise<[MessageReader, MessageWriter]>; close(): void }): Promise { return new Promise((resolve, reject) => { - childProcess.once('exit', (code, signal) => { + const removeListeners = () => { + childProcess.removeListener('exit', onExit); + childProcess.removeListener('error', onError); + }; + const onExit = (code: number | null, signal: NodeJS.Signals | null) => { + removeListeners(); // The server never connected, so stop listening to release the pipe / port. - transport.dispose(); + transport.close(); reject(new Error(`Server process exited before the connection could be established (code: ${code}, signal: ${signal}).`)); - }); + }; // A failed spawn (e.g. invalid execPath) emits 'error' and never 'exit'. - childProcess.once('error', (error) => { - transport.dispose(); + const onError = (error: Error) => { + removeListeners(); + transport.close(); + reject(error); + }; + childProcess.once('exit', onExit); + childProcess.once('error', onError); + transport.onConnected().then((protocol) => { + removeListeners(); + resolve({ reader: protocol[0], writer: protocol[1] }); + }, (error) => { + removeListeners(); reject(error); }); - transport.onConnected().then((protocol) => resolve({ reader: protocol[0], writer: protocol[1] }), reject); }); } diff --git a/jsonrpc/src/node/main.ts b/jsonrpc/src/node/main.ts index 6db327273..fbc64a1b5 100644 --- a/jsonrpc/src/node/main.ts +++ b/jsonrpc/src/node/main.ts @@ -196,10 +196,12 @@ export function generateRandomPipeName(): string { export interface PipeTransport { onConnected(): Promise<[MessageReader, MessageWriter]>; /** - * Stops listening for a connection. Use this to release the pipe if the - * other side never connects (e.g. the server process exited early). + * Closes the underlying server if a client hasn't connected yet. Callers + * should invoke this if they abandon the transport before `onConnected` + * resolves (for example because the server process failed to start) to + * avoid leaking the listening pipe. */ - dispose(): void; + close(): void; } export function createClientPipeTransport(pipeName: string, encoding: RAL.MessageBufferEncoding = 'utf-8'): Promise { @@ -220,11 +222,7 @@ export function createClientPipeTransport(pipeName: string, encoding: RAL.Messag server.removeListener('error', reject); resolve({ onConnected: () => { return connected; }, - dispose: () => { - if (server.listening) { - server.close(); - } - } + close: () => { server.close(); } }); }); }); @@ -242,10 +240,12 @@ export interface SocketTransport { port(): number; onConnected(): Promise<[MessageReader, MessageWriter]>; /** - * Stops listening for a connection. Use this to release the port if the - * other side never connects (e.g. the server process exited early). + * Closes the underlying server if a client hasn't connected yet. Callers + * should invoke this if they abandon the transport before `onConnected` + * resolves (for example because the server process failed to start) to + * avoid leaking the listening socket. */ - dispose(): void; + close(): void; } export function createClientSocketTransport(port: number, encoding: RAL.MessageBufferEncoding = 'utf-8'): Promise { @@ -273,11 +273,7 @@ export function createClientSocketTransport(port: number, encoding: RAL.MessageB resolve({ port: () => boundPort, onConnected: () => { return connected; }, - dispose: () => { - if (server.listening) { - server.close(); - } - } + close: () => { server.close(); } }); }); }); diff --git a/jsonrpc/src/node/test/connection.test.ts b/jsonrpc/src/node/test/connection.test.ts index 6dbdc2ceb..ee4c514d2 100644 --- a/jsonrpc/src/node/test/connection.test.ts +++ b/jsonrpc/src/node/test/connection.test.ts @@ -104,30 +104,38 @@ suite('Connection', () => { assert.ok(bound > 0, `expected a positive bound port, got ${bound}`); }); - test('createClientSocketTransport releases the port on dispose', async () => { + test('createClientSocketTransport close closes the listening socket before a client connects', async () => { const net = await import('net'); const transport = await hostConnection.createClientSocketTransport(0); const bound = transport.port(); - transport.dispose(); - const probe = net.createServer(); + + transport.close(); + await new Promise((resolve, reject) => { - probe.on('error', reject); - probe.listen(bound, '127.0.0.1', () => resolve()); + const client = net.connect(bound, '127.0.0.1'); + client.on('connect', () => { + client.destroy(); + reject(new Error('expected the connection to be refused after close')); + }); + client.on('error', () => resolve()); }); - await new Promise(resolve => probe.close(() => resolve())); }); - test('createClientPipeTransport releases the pipe on dispose', async () => { + test('createClientPipeTransport close closes the listening pipe before a client connects', async () => { const net = await import('net'); const pipeName = hostConnection.generateRandomPipeName(); const transport = await hostConnection.createClientPipeTransport(pipeName); - transport.dispose(); - const probe = net.createServer(); + + transport.close(); + await new Promise((resolve, reject) => { - probe.on('error', reject); - probe.listen(pipeName, () => resolve()); + const client = net.createConnection(pipeName); + client.on('connect', () => { + client.destroy(); + reject(new Error('expected the connection to be refused after close')); + }); + client.on('error', () => resolve()); }); - await new Promise(resolve => probe.close(() => resolve())); }); test('Test Duplex Stream Connection', (done) => { From 31ad4508bec86b260c548276f9d54c58bdf0dcf6 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Thu, 1 Oct 2026 05:57:33 -0700 Subject: [PATCH 5/5] client-node-tests: cover the Executable branch when the server exits before connecting Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- client-node-tests/src/integration.test.ts | 32 +++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/client-node-tests/src/integration.test.ts b/client-node-tests/src/integration.test.ts index a18a8dd5c..afa2afc14 100644 --- a/client-node-tests/src/integration.test.ts +++ b/client-node-tests/src/integration.test.ts @@ -323,6 +323,22 @@ suite('Socket transport', () => { await client.start(); }, /Server process exited before the connection could be established/); }).timeout(5000); + + test('start() rejects if the executable server process exits before connecting', async () => { + const serverOptions: lsclient.ServerOptions = { + command: process.execPath, + args: [path.join(__dirname, './servers/exitBeforeConnectServer.js')], + // The tests run inside Electron, so process.execPath must be told to behave like node. + options: { env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' } }, + transport: { kind: lsclient.TransportKind.socket, port: 0 }, + }; + const clientOptions: lsclient.LanguageClientOptions = {}; + const client = new lsclient.LanguageClient('socket executable exit before connect', 'Socket Transport (executable exits before connecting)', serverOptions, clientOptions); + // Covers the Executable branch (the command + socket path from #1586), which is separate from the NodeModule branch above. + await assert.rejects(async () => { + await client.start(); + }, /Server process exited before the connection could be established/); + }).timeout(5000); }); suite('Pipe transport', () => { @@ -340,6 +356,22 @@ suite('Pipe transport', () => { await client.start(); }, /Server process exited before the connection could be established/); }).timeout(5000); + + test('start() rejects if the executable server process exits before connecting', async () => { + const serverOptions: lsclient.ServerOptions = { + command: process.execPath, + args: [path.join(__dirname, './servers/exitBeforeConnectServer.js')], + // The tests run inside Electron, so process.execPath must be told to behave like node. + options: { env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' } }, + transport: lsclient.TransportKind.pipe, + }; + const clientOptions: lsclient.LanguageClientOptions = {}; + const client = new lsclient.LanguageClient('pipe executable exit before connect', 'Pipe Transport (executable exits before connecting)', serverOptions, clientOptions); + // Covers the Executable branch (the command + pipe path from #1586), which is separate from the NodeModule branch above. + await assert.rejects(async () => { + await client.start(); + }, /Server process exited before the connection could be established/); + }).timeout(5000); }); suite('Client integration', () => {