Skip to content

Commit

Permalink
fix(nxls): clean up connection & ipc channel when shutting down nxls …
Browse files Browse the repository at this point in the history
…to prevent it from staying open (#2353)
  • Loading branch information
MaxKless authored Dec 10, 2024
1 parent c7bc53d commit 6357105
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 145 deletions.
4 changes: 2 additions & 2 deletions apps/nxls-e2e/src/nxls-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
StreamMessageWriter,
} from 'vscode-languageserver/node';

import { killTree } from '@nx-console/shared/utils';
import { defaultVersion } from './utils';
import { killGroup } from '@nx-console/shared/utils';

export class NxlsWrapper {
private cwd?: string;
Expand Down Expand Up @@ -147,7 +147,7 @@ export class NxlsWrapper {

if (this.process?.pid) {
try {
killTree(this.process.pid, 'SIGKILL');
killGroup(this.process.pid);
} catch (e) {
console.log(`NXLS WRAPPER: ${e}`);
}
Expand Down
36 changes: 21 additions & 15 deletions apps/nxls/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
completionHandler,
configureSchemaForProject,
configureSchemas,
getCompletionItems,
projectSchemaIsRegistered,
resetInferencePluginsCompletionCache,
} from '@nx-console/language-server/capabilities/code-completion';
Expand Down Expand Up @@ -44,7 +43,6 @@ import {
getJsonLanguageService,
getLanguageModelCache,
lspLogger,
mergeArrays,
setLspLogger,
} from '@nx-console/language-server/utils';
import {
Expand Down Expand Up @@ -80,13 +78,8 @@ import {
} from '@nx-console/language-server/workspace';
import { GeneratorSchema } from '@nx-console/shared/generate-ui-types';
import { NxWorkspace } from '@nx-console/shared/types';
import { formatError, killTree } from '@nx-console/shared/utils';
import { dirname, relative } from 'node:path';
import {
ClientCapabilities,
CompletionList,
TextDocument,
} from 'vscode-json-languageservice';
import { formatError, killGroup } from '@nx-console/shared/utils';
import { ClientCapabilities, TextDocument } from 'vscode-json-languageservice';
import {
CreateFilesParams,
DeleteFilesParams,
Expand Down Expand Up @@ -148,7 +141,6 @@ connection.onInitialize(async (params) => {
CLIENT_CAPABILITIES = params.capabilities;

await configureSchemas(WORKING_PATH, CLIENT_CAPABILITIES);

unregisterFileWatcher = await languageServerWatcher(
WORKING_PATH,
async () => {
Expand Down Expand Up @@ -300,11 +292,6 @@ connection.onShutdown(async () => {
jsonDocumentMapper.dispose();
});

connection.onExit(() => {
connection.dispose();
killTree(process.pid);
});

connection.onRequest(NxStopDaemonRequest, async () => {
if (!WORKING_PATH) {
return new ResponseError(1000, 'Unable to get Nx info: no workspace path');
Expand Down Expand Up @@ -663,4 +650,23 @@ function getJsonDocument(document: TextDocument) {
}

ensureOnlyJsonRpcStdout();

const exitHandler = () => {
process.off('SIGTERM', exitHandler);

try {
connection.dispose();
} catch (e) {
// noop
}

if (process.connected) {
process.disconnect();
}

killGroup(process.pid);
};
process.on('SIGTERM', exitHandler);

connection.onExit(exitHandler);
connection.listen();
6 changes: 3 additions & 3 deletions apps/vscode/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {

import {
checkIsNxWorkspace,
killTree,
killGroup,
withTimeout,
} from '@nx-console/shared/utils';
import {
Expand Down Expand Up @@ -137,12 +137,12 @@ export async function deactivate() {

const nxlsPid = getNxlsClient()?.getNxlsPid();
if (nxlsPid) {
killTree(nxlsPid, 'SIGINT');
killGroup(nxlsPid);
}

getTelemetry().logUsage('extension-deactivate');

killTree(process.pid, 'SIGTERM');
killGroup(process.pid);
}

// -----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion libs/shared/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ export { checkIsNxWorkspace } from './lib/check-is-nx-workspace';
export { getNxExecutionCommand } from './lib/get-nx-execution-command';
export * from './lib/parse-target-string';
export * from './lib/utils';
export * from './lib/kill-tree';
export * from './lib/cipe';
export * from './lib/package-manager-command';
export * from './lib/kill-group';
17 changes: 17 additions & 0 deletions libs/shared/utils/src/lib/kill-group.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { execSync } from 'child_process';

export function killGroup(pid: number, signal: NodeJS.Signals = 'SIGTERM') {
if (process.platform === 'win32') {
try {
execSync('taskkill /pid ' + pid + ' /T /F', {
windowsHide: true,
});
} catch (err: any) {
if (err?.status !== 128) {
throw err;
}
}
} else {
process.kill(-pid, signal);
}
}
122 changes: 0 additions & 122 deletions libs/shared/utils/src/lib/kill-tree.ts

This file was deleted.

4 changes: 2 additions & 2 deletions libs/vscode/lsp-client/src/lib/nxls-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
NxWorkspaceRefreshNotification,
NxWorkspaceRefreshStartedNotification,
} from '@nx-console/language-server/types';
import { killTree } from '@nx-console/shared/utils';
import {
getNxlsOutputChannel,
getOutputChannel,
Expand All @@ -25,6 +24,7 @@ import {
} from 'vscode-languageclient/node';
import { createActor, fromPromise, waitFor } from 'xstate';
import { nxlsClientStateMachine } from './nxls-client-state-machine';
import { killGroup } from '@nx-console/shared/utils';

let _nxlsClient: NxlsClient | undefined;

Expand Down Expand Up @@ -296,7 +296,7 @@ export class NxlsClient {
// timeout, kill the process forcefully instead
const pid = this.actor.getSnapshot().context.nxlsPid;
if (pid) {
killTree(pid, 'SIGTERM');
killGroup(pid);
}
}
}
Expand Down

0 comments on commit 6357105

Please sign in to comment.