Skip to content

Commit 7b654ad

Browse files
authored
Inline Linux/macOS process-tree termination in client dispose path (#1761)
* Initial plan * Inline process termination script and remove external shell dependency * Address review feedback for termination script and test messaging * Add pid validation guard for inline termination script * Add typed serverProcess getter and remove any cast in shutdown-timeout test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent f373db6 commit 7b654ad

5 files changed

Lines changed: 47 additions & 23 deletions

File tree

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,25 @@ function isFullDocumentDiagnosticReport(value: vsdiag.DocumentDiagnosticReport):
8989
assert.ok(value.kind === vsdiag.DocumentDiagnosticReportKind.full);
9090
}
9191

92+
function processExists(pid: number): boolean {
93+
try {
94+
process.kill(pid, 0);
95+
return true;
96+
} catch {
97+
return false;
98+
}
99+
}
100+
101+
async function waitForProcessExit(pid: number, timeout: number): Promise<void> {
102+
const start = Date.now();
103+
while (processExists(pid)) {
104+
if (Date.now() - start > timeout) {
105+
throw new Error(`Process ${pid} did not exit within ${timeout} ms.`);
106+
}
107+
await new Promise((resolve) => setTimeout(resolve, 50));
108+
}
109+
}
110+
92111
interface FoldingRangeTestFeature {
93112
getRegistration(documentSelector: lsclient.DocumentSelector | undefined, capability: undefined | lsclient.FoldingRangeOptions | (lsclient.FoldingRangeRegistrationOptions & lsclient.StaticRegistrationOptions)): [string | undefined, (lsclient.FoldingRangeRegistrationOptions & { documentSelector: lsclient.DocumentSelector }) | undefined];
94113
getRegistrationOptions(documentSelector: lsclient.DocumentSelector | undefined, capability: undefined | lsclient.FoldingRangeOptions) : (lsclient.FoldingRangeRegistrationOptions & { documentSelector: lsclient.DocumentSelector }) | undefined;
@@ -2002,10 +2021,13 @@ suite('Server tests', () => {
20022021
const clientOptions: lsclient.LanguageClientOptions = {};
20032022
const client = new lsclient.LanguageClient('test svr', 'Test Language Server', serverOptions, clientOptions);
20042023
await client.start();
2024+
const serverPid = client.serverProcess?.pid;
2025+
assert.strictEqual(typeof serverPid, 'number');
20052026

20062027
await assert.rejects(async () => {
20072028
await client.stop(100);
20082029
}, /Stopping the server timed out/);
2030+
await waitForProcessExit(serverPid!, 10000);
20092031
});
20102032

20112033
test('Server can\'t be stopped right after start', async() => {

client/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,14 @@
4848
"scripts": {
4949
"prepublishOnly": "echo \"⛔ Can only publish from a secure pipeline ⛔\" && node ../build/npm/fail",
5050
"prepack": "npm run all:publish",
51-
"compile": "tsc -b ./tsconfig.json && shx cp src/node/terminateProcess.sh lib/node/terminateProcess.sh",
51+
"compile": "tsc -b ./tsconfig.json",
5252
"compile:clean": "git clean -xfd . && npm install && npm run clean && npm run compile",
5353
"watch": "tsc -b ./tsconfig.watch.json -w",
5454
"lint": "eslint src",
5555
"test": "cd ../client-node-tests && npm test && cd ../client",
5656
"clean": "rimraf lib",
5757
"all": "npm run clean && npm run compile && npm run lint && npm test",
58-
"compile:publish": "tsc -b ./tsconfig.publish.json && shx cp src/node/terminateProcess.sh lib/node/terminateProcess.sh",
58+
"compile:publish": "tsc -b ./tsconfig.publish.json",
5959
"all:publish": "git clean -xfd . && npm install && npm run updateVSCodeVersion && npm run compile:publish && npm run lint && cd ../client-node-tests && npm run all:publish && cd ..",
6060
"preversion": "npm test",
6161
"updateVSCodeVersion": "node ./bin/updateVSCode.js",

client/src/node/main.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,10 @@ export class LanguageClient extends BaseLanguageClient {
189189
return this._isInDebugMode;
190190
}
191191

192+
public get serverProcess(): ChildProcess | undefined {
193+
return this._serverProcess;
194+
}
195+
192196
public async restart(): Promise<void> {
193197
await this.stop();
194198
// We are in debug mode. Wait a little before we restart

client/src/node/processes.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
import * as cp from 'child_process';
77
import ChildProcess = cp.ChildProcess;
88

9-
import { join } from 'path';
10-
119
const isWindows = (process.platform === 'win32');
1210
const isMacintosh = (process.platform === 'darwin');
1311
const isLinux = (process.platform === 'linux');
@@ -30,8 +28,24 @@ export function terminate(process: ChildProcess & { pid: number }, cwd?: string)
3028
}
3129
} else if (isLinux || isMacintosh) {
3230
try {
33-
const cmd = join(__dirname, 'terminateProcess.sh');
34-
const result = (<any>cp).spawnSync(cmd, [process.pid.toString()]);
31+
const pid = process.pid.toString();
32+
if (!/^\d+$/.test(pid)) {
33+
return false;
34+
}
35+
const script = `
36+
terminateTree() {
37+
for cpid in $(pgrep -P "$1"); do
38+
terminateTree "$cpid"
39+
done
40+
kill -9 "$1" > /dev/null 2>&1
41+
}
42+
43+
terminateTree "${pid}"
44+
`;
45+
const result = (<any>cp).spawnSync('/bin/sh', [], {
46+
input: script,
47+
stdio: ['pipe', 'inherit', 'inherit']
48+
});
3549
return result.error ? false : true;
3650
} catch (err) {
3751
return false;
@@ -40,4 +54,4 @@ export function terminate(process: ChildProcess & { pid: number }, cwd?: string)
4054
process.kill('SIGKILL');
4155
return true;
4256
}
43-
}
57+
}

client/src/node/terminateProcess.sh

Lines changed: 0 additions & 16 deletions
This file was deleted.

0 commit comments

Comments
 (0)