Fix process cancelation (#3131)
* Fix process cancelation
Processes were not being killed due to the path to the external scripts
being incorrect. Fix this by relying on the tree-kill package rather
than shelling out to an external script to kill a process and its
children.
Also add sinon as a dev dependency for testing purposes.
Updates #3044
* Use tree-kill directly in the debug adapter.
We cannot take a dependency on util, which depends on the vscode module
because the debug adapter runs in a process that doesn't have access
to the vscode module.
Co-authored-by: Ramya Achutha Rao <ramyar@microsoft.com>
diff --git a/package-lock.json b/package-lock.json
index 8673f8c..7de5183 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -24,6 +24,51 @@
"js-tokens": "^4.0.0"
}
},
+ "@sinonjs/commons": {
+ "version": "1.7.1",
+ "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.7.1.tgz",
+ "integrity": "sha512-Debi3Baff1Qu1Unc3mjJ96MgpbwTn43S1+9yJ0llWygPwDNu2aaWBD6yc9y/Z8XDRNhx7U+u2UDg2OGQXkclUQ==",
+ "dev": true,
+ "requires": {
+ "type-detect": "4.0.8"
+ }
+ },
+ "@sinonjs/fake-timers": {
+ "version": "6.0.1",
+ "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-6.0.1.tgz",
+ "integrity": "sha512-MZPUxrmFubI36XS1DI3qmI0YdN1gks62JtFZvxR67ljjSNCeK6U08Zx4msEWOXuofgqUt6zPHSi1H9fbjR/NRA==",
+ "dev": true,
+ "requires": {
+ "@sinonjs/commons": "^1.7.0"
+ }
+ },
+ "@sinonjs/formatio": {
+ "version": "5.0.1",
+ "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-5.0.1.tgz",
+ "integrity": "sha512-KaiQ5pBf1MpS09MuA0kp6KBQt2JUOQycqVG1NZXvzeaXe5LGFqAKueIS0bw4w0P9r7KuBSVdUk5QjXsUdu2CxQ==",
+ "dev": true,
+ "requires": {
+ "@sinonjs/commons": "^1",
+ "@sinonjs/samsam": "^5.0.2"
+ }
+ },
+ "@sinonjs/samsam": {
+ "version": "5.0.3",
+ "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-5.0.3.tgz",
+ "integrity": "sha512-QucHkc2uMJ0pFGjJUDP3F9dq5dx8QIaqISl9QgwLOh6P9yv877uONPGXh/OH/0zmM3tW1JjuJltAZV2l7zU+uQ==",
+ "dev": true,
+ "requires": {
+ "@sinonjs/commons": "^1.6.0",
+ "lodash.get": "^4.4.2",
+ "type-detect": "^4.0.8"
+ }
+ },
+ "@sinonjs/text-encoding": {
+ "version": "0.7.1",
+ "resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz",
+ "integrity": "sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==",
+ "dev": true
+ },
"@types/events": {
"version": "3.0.0",
"resolved": "https://registry.npmjs.org/@types/events/-/events-3.0.0.tgz",
@@ -74,6 +119,12 @@
"integrity": "sha512-1OzrNb4RuAzIT7wHSsgZRlMBlNsJl+do6UblR7JMW4oB7bbR+uBEYtUh7gEc/jM84GGilh68lSOokyM/zNUlBA==",
"dev": true
},
+ "@types/sinon": {
+ "version": "7.5.2",
+ "resolved": "https://registry.npmjs.org/@types/sinon/-/sinon-7.5.2.tgz",
+ "integrity": "sha512-T+m89VdXj/eidZyejvmoP9jivXgBDdkOSBVQjU9kF349NEx10QdPNGxHeZUaj1IlJ32/ewdyXJjnJxyxJroYwg==",
+ "dev": true
+ },
"@types/vscode": {
"version": "1.41.0",
"resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.41.0.tgz",
@@ -823,6 +874,12 @@
"resolved": "https://registry.npmjs.org/is-typedarray/-/is-typedarray-1.0.0.tgz",
"integrity": "sha1-5HnICFjfDBsR3dppQPlgEfzaSpo="
},
+ "isarray": {
+ "version": "0.0.1",
+ "resolved": "https://registry.npmjs.org/isarray/-/isarray-0.0.1.tgz",
+ "integrity": "sha1-ihis/Kmo9Bd+Cav8YDiTmwXR7t8=",
+ "dev": true
+ },
"isexe": {
"version": "2.0.0",
"resolved": "https://registry.npmjs.org/isexe/-/isexe-2.0.0.tgz",
@@ -909,6 +966,12 @@
"verror": "1.10.0"
}
},
+ "just-extend": {
+ "version": "4.1.0",
+ "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.1.0.tgz",
+ "integrity": "sha512-ApcjaOdVTJ7y4r08xI5wIqpvwS48Q0PBG4DJROcEkH1f8MdAiNFyFxz3xoL0LWAVwjrwPYZdVHHxhRHcx/uGLA==",
+ "dev": true
+ },
"locate-path": {
"version": "3.0.0",
"resolved": "https://registry.npmjs.org/locate-path/-/locate-path-3.0.0.tgz",
@@ -924,6 +987,12 @@
"resolved": "https://registry.npmjs.org/lodash/-/lodash-3.10.1.tgz",
"integrity": "sha1-W/Rejkm6QYnhfUgnid/RW9FAt7Y="
},
+ "lodash.get": {
+ "version": "4.4.2",
+ "resolved": "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz",
+ "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=",
+ "dev": true
+ },
"log-symbols": {
"version": "2.2.0",
"resolved": "https://registry.npmjs.org/log-symbols/-/log-symbols-2.2.0.tgz",
@@ -1046,6 +1115,19 @@
"resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz",
"integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g="
},
+ "nise": {
+ "version": "4.0.3",
+ "resolved": "https://registry.npmjs.org/nise/-/nise-4.0.3.tgz",
+ "integrity": "sha512-EGlhjm7/4KvmmE6B/UFsKh7eHykRl9VH+au8dduHLCyWUO/hr7+N+WtTvDUwc9zHuM1IaIJs/0lQ6Ag1jDkQSg==",
+ "dev": true,
+ "requires": {
+ "@sinonjs/commons": "^1.7.0",
+ "@sinonjs/fake-timers": "^6.0.0",
+ "@sinonjs/text-encoding": "^0.7.1",
+ "just-extend": "^4.0.2",
+ "path-to-regexp": "^1.7.0"
+ }
+ },
"node-environment-flags": {
"version": "1.0.5",
"resolved": "https://registry.npmjs.org/node-environment-flags/-/node-environment-flags-1.0.5.tgz",
@@ -1159,6 +1241,15 @@
"integrity": "sha512-GSmOT2EbHrINBf9SR7CDELwlJ8AENk3Qn7OikK4nFYAu3Ote2+JYNVvkpAEQm3/TLNEJFD/xZJjzyxg3KBWOzw==",
"dev": true
},
+ "path-to-regexp": {
+ "version": "1.8.0",
+ "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.8.0.tgz",
+ "integrity": "sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==",
+ "dev": true,
+ "requires": {
+ "isarray": "0.0.1"
+ }
+ },
"performance-now": {
"version": "2.1.0",
"resolved": "https://registry.npmjs.org/performance-now/-/performance-now-2.1.0.tgz",
@@ -1268,6 +1359,38 @@
"resolved": "https://registry.npmjs.org/shimmer/-/shimmer-1.2.1.tgz",
"integrity": "sha512-sQTKC1Re/rM6XyFM6fIAGHRPVGvyXfgzIDvzoq608vM+jeyVD0Tu1E6Np0Kc2zAIFWIj963V2800iF/9LPieQw=="
},
+ "sinon": {
+ "version": "9.0.1",
+ "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.0.1.tgz",
+ "integrity": "sha512-iTTyiQo5T94jrOx7X7QLBZyucUJ2WvL9J13+96HMfm2CGoJYbIPqRfl6wgNcqmzk0DI28jeGx5bUTXizkrqBmg==",
+ "dev": true,
+ "requires": {
+ "@sinonjs/commons": "^1.7.0",
+ "@sinonjs/fake-timers": "^6.0.0",
+ "@sinonjs/formatio": "^5.0.1",
+ "@sinonjs/samsam": "^5.0.3",
+ "diff": "^4.0.2",
+ "nise": "^4.0.1",
+ "supports-color": "^7.1.0"
+ },
+ "dependencies": {
+ "has-flag": {
+ "version": "4.0.0",
+ "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz",
+ "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==",
+ "dev": true
+ },
+ "supports-color": {
+ "version": "7.1.0",
+ "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.1.0.tgz",
+ "integrity": "sha512-oRSIpR8pxT1Wr2FquTNnGet79b3BWljqOuoW/h4oBhxJ/HUbX5nX6JSruTkvXDCFMwDPvsaTTbvMLKZWSy0R5g==",
+ "dev": true,
+ "requires": {
+ "has-flag": "^4.0.0"
+ }
+ }
+ }
+ },
"sprintf-js": {
"version": "1.0.3",
"resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz",
@@ -1365,6 +1488,11 @@
}
}
},
+ "tree-kill": {
+ "version": "1.2.2",
+ "resolved": "https://registry.npmjs.org/tree-kill/-/tree-kill-1.2.2.tgz",
+ "integrity": "sha512-L0Orpi8qGpRG//Nd+H90vFB+3iHnue1zSSGmNOOCh1GLJ7rUKVwV2HvijphGQS2UmhUZewS9VgvxYIdgr+fG1A=="
+ },
"tslib": {
"version": "1.10.0",
"resolved": "https://registry.npmjs.org/tslib/-/tslib-1.10.0.tgz",
@@ -1422,6 +1550,12 @@
"resolved": "https://registry.npmjs.org/tweetnacl/-/tweetnacl-0.14.5.tgz",
"integrity": "sha1-WuaBd/GS1EViadEIr6k/+HQ/T2Q="
},
+ "type-detect": {
+ "version": "4.0.8",
+ "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-4.0.8.tgz",
+ "integrity": "sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g==",
+ "dev": true
+ },
"typescript": {
"version": "3.7.5",
"resolved": "https://registry.npmjs.org/typescript/-/typescript-3.7.5.tgz",
diff --git a/package.json b/package.json
index 8c05098..b25708d 100644
--- a/package.json
+++ b/package.json
@@ -44,6 +44,7 @@
"json-rpc2": "^1.0.2",
"moment": "^2.24.0",
"semver": "^6.3.0",
+ "tree-kill": "^1.2.2",
"vscode-debugadapter": "^1.36.0",
"vscode-debugprotocol": "^1.36.0",
"vscode-extension-telemetry": "^0.1.2",
@@ -56,14 +57,16 @@
"@types/mocha": "^5.2.7",
"@types/node": "^12.7.2",
"@types/semver": "^6.0.1",
+ "@types/sinon": "^7.5.2",
"@types/vscode": "^1.25.0",
"fs-extra": "^8.1.0",
"glob": "^7.1.4",
"mocha": "^6.2.0",
+ "prettier": "^1.16.4",
+ "sinon": "^9.0.1",
"tslint": "^5.19.0",
"typescript": "^3.7.2",
- "vscode-test": "^1.2.3",
- "prettier": "^1.16.4"
+ "vscode-test": "^1.2.3"
},
"engines": {
"vscode": "^1.41.0"
diff --git a/scripts/terminateProcess.sh b/scripts/terminateProcess.sh
deleted file mode 100755
index 9b06884..0000000
--- a/scripts/terminateProcess.sh
+++ /dev/null
@@ -1,12 +0,0 @@
-#!/bin/bash
-
-terminateTree() {
- for cpid in $(/usr/bin/pgrep -P $1); do
- terminateTree $cpid
- done
- kill -9 $1 > /dev/null 2>&1
-}
-
-for pid in $*; do
- terminateTree $pid
-done
diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts
index 5ddfc90..4310ed7 100644
--- a/src/debugAdapter/goDebug.ts
+++ b/src/debugAdapter/goDebug.ts
@@ -9,9 +9,9 @@
import { Client, RPCConnection } from 'json-rpc2';
import * as os from 'os';
import * as path from 'path';
+import kill = require('tree-kill');
import * as util from 'util';
import {
- Breakpoint,
DebugSession,
Handles,
InitializedEvent,
@@ -661,7 +661,7 @@
const isLocalDebugging: boolean = this.request === 'launch' && !!this.debugProcess;
const forceCleanup = async () => {
- killTree(this.debugProcess.pid);
+ kill(this.debugProcess.pid, (err) => console.log('Error killing debug process: ' + err));
await removeFile(this.localDebugeePath);
};
return new Promise(async (resolve) => {
@@ -1927,28 +1927,6 @@
return Math.floor(Math.random() * (high - low) + low);
}
-function killTree(processId: number): void {
- if (process.platform === 'win32') {
- const TASK_KILL = 'C:\\Windows\\System32\\taskkill.exe';
-
- // when killing a process in Windows its child processes are *not* killed but become root processes.
- // Therefore we use TASKKILL.EXE
- try {
- execSync(`${TASK_KILL} /F /T /PID ${processId}`);
- } catch (err) {
- logError(`Error killing process tree: ${err.toString() || ''}`);
- }
- } else {
- // on linux and OS X we kill all direct and indirect child processes as well
- try {
- const cmd = path.join(__dirname, '../../../scripts/terminateProcess.sh');
- spawnSync(cmd, [processId.toString()]);
- } catch (err) {
- logError(`Error killing process tree: ${err.toString() || ''}`);
- }
- }
-}
-
async function removeFile(filePath: string): Promise<void> {
try {
const fileExists = await fsAccess(filePath)
diff --git a/src/util.ts b/src/util.ts
index b61e51b..6c27721 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -8,6 +8,7 @@
import os = require('os');
import path = require('path');
import semver = require('semver');
+import kill = require('tree-kill');
import vscode = require('vscode');
import { NearestNeighborDict, Node } from './avlTree';
import { buildDiagnosticCollection, lintDiagnosticCollection, vetDiagnosticCollection } from './goMain';
@@ -867,27 +868,13 @@
}
}
-export function killTree(processId: number): void {
- if (process.platform === 'win32') {
- const TASK_KILL = 'C:\\Windows\\System32\\taskkill.exe';
-
- // when killing a process in Windows its child processes are *not* killed but become root processes.
- // Therefore we use TASKKILL.EXE
- try {
- cp.execSync(`${TASK_KILL} /F /T /PID ${processId}`);
- } catch (err) {
+export const killTree = (processId: number): void => {
+ kill(processId, (err) => {
+ if (err) {
console.log('Error killing process tree: ' + err);
}
- } else {
- // on linux and OS X we kill all direct and indirect child processes as well
- try {
- const cmd = path.join(__dirname, '../../../scripts/terminateProcess.sh');
- cp.spawnSync(cmd, [processId.toString()]);
- } catch (err) {
- console.log('Error killing process tree: ' + err);
- }
- }
-}
+ });
+};
export function makeMemoizedByteOffsetConverter(buffer: Buffer): (byteOffset: number) => number {
const defaultValue = new Node<number, number>(0, 0); // 0 bytes will always be 0 characters
diff --git a/test/integration/extension.test.ts b/test/integration/extension.test.ts
index 9c68493..613d743 100644
--- a/test/integration/extension.test.ts
+++ b/test/integration/extension.test.ts
@@ -7,6 +7,7 @@
import cp = require('child_process');
import * as fs from 'fs-extra';
import * as path from 'path';
+import * as sinon from 'sinon';
import * as vscode from 'vscode';
import { FilePatch, getEdits, getEditsFromUnifiedDiffStr } from '../../src/diffUtils';
import { check } from '../../src/goCheck';
@@ -19,6 +20,7 @@
generateTestCurrentPackage
} from '../../src/goGenerateTests';
import { getTextEditForAddImport, listPackages } from '../../src/goImport';
+import { goLint } from '../../src/goLint';
import { updateGoPathGoRootFromConfig } from '../../src/goInstallTools';
import { documentSymbols, GoDocumentSymbolProvider, GoOutlineImportsOptions } from '../../src/goOutline';
import { getAllPackages } from '../../src/goPackages';
@@ -190,6 +192,10 @@
fs.removeSync(repoPath);
});
+ teardown(() => {
+ sinon.restore();
+ });
+
async function testDefinitionProvider(goConfig: vscode.WorkspaceConfiguration): Promise<any> {
const provider = new GoDefinitionProvider(goConfig);
const uri = vscode.Uri.file(path.join(fixturePath, 'baseTest', 'test.go'));
@@ -430,6 +436,29 @@
await testHoverProvider(config, testCases);
});
+ test('Linting - concurrent process cancelation', async () => {
+ const util = require('../../src/util');
+ sinon.spy(util, 'runTool');
+ sinon.spy(util, 'killTree');
+
+ const config = Object.create(vscode.workspace.getConfiguration('go'), {
+ vetOnSave: { value: 'package' },
+ vetFlags: { value: ['-all'] },
+ buildOnSave: { value: 'package' },
+ lintOnSave: { value: 'package' },
+ // simulate a long running lint process by sleeping for a couple seconds
+ lintTool: { value: 'sleep' },
+ lintFlags: { value: ['2'] }
+ });
+
+ const results = await Promise.all([
+ goLint(vscode.Uri.file(path.join(fixturePath, 'linterTest', 'linter_1.go')), config),
+ goLint(vscode.Uri.file(path.join(fixturePath, 'linterTest', 'linter_2.go')), config)
+ ]);
+ assert.equal(util.runTool.callCount, 2, 'should have launched 2 lint jobs');
+ assert.equal(util.killTree.callCount, 1, 'should have killed 1 lint job before launching the next');
+ });
+
test('Error checking', async () => {
const config = Object.create(vscode.workspace.getConfiguration('go'), {
vetOnSave: { value: 'package' },
