From cebd1de0aa8b3d1f4baabd8ce4f91cf3821ed9af Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 24 Oct 2018 21:09:45 -0700 Subject: [PATCH 01/12] Remove multiprocess flag --- .../debugAdapter/DebugClients/localDebugClientV2.ts | 7 +------ .../debugger/extension/hooks/processTerminationHandler.ts | 5 +++-- src/client/debugger/types.ts | 1 - 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/client/debugger/debugAdapter/DebugClients/localDebugClientV2.ts b/src/client/debugger/debugAdapter/DebugClients/localDebugClientV2.ts index 7e528d037de7..ac0f40ffa508 100644 --- a/src/client/debugger/debugAdapter/DebugClients/localDebugClientV2.ts +++ b/src/client/debugger/debugAdapter/DebugClients/localDebugClientV2.ts @@ -4,7 +4,7 @@ 'use strict'; import { DebugSession } from 'vscode-debugadapter'; -import { IKnownLaunchRequestArguments, LaunchRequestArguments } from '../../types'; +import { LaunchRequestArguments } from '../../types'; import { IDebugLauncherScriptProvider } from '../types'; import { LocalDebugClient } from './LocalDebugClient'; @@ -18,11 +18,6 @@ export class LocalDebugClientV2 extends LocalDebugClient { if (this.args.noDebug) { additionalPtvsdArgs.push('--nodebug'); } - const multiProcessPropety: keyof IKnownLaunchRequestArguments = 'multiProcess'; - const multiProcess = (this.args as Object).hasOwnProperty(multiProcessPropety) ? this.args.multiProcess : true; - if (multiProcess) { - additionalPtvsdArgs.push('--multiprocess'); - } return [launcher, ...additionalPtvsdArgs, '--client', '--host', 'localhost', '--port', debugPort.toString()]; } protected buildStandardArguments() { diff --git a/src/client/debugger/extension/hooks/processTerminationHandler.ts b/src/client/debugger/extension/hooks/processTerminationHandler.ts index 9e24f291b528..7c219bc2688a 100644 --- a/src/client/debugger/extension/hooks/processTerminationHandler.ts +++ b/src/client/debugger/extension/hooks/processTerminationHandler.ts @@ -92,8 +92,9 @@ export class ProcessTerminationEventHandler implements IDebugSessionEventHandler if (!data.rootProcessId || data.rootStartRequest.arguments.request !== 'launch') { return; } - this.processTermination.trackProcess(data.processId, data.parentProcessId); - this.processTermination.trackProcess(data.processId, data.rootProcessId); + this.processTermination.trackProcess(data.parentProcessId); + // this.processTermination.trackProcess(data.processId, data.parentProcessId); + // this.processTermination.trackProcess(data.processId, data.rootProcessId); this.procecssIdsTrackedToKill.add(data.processId); this.procecssIdsTrackedToKill.add(data.parentProcessId); diff --git a/src/client/debugger/types.ts b/src/client/debugger/types.ts index 4367c1e0e17d..2d34903228ab 100644 --- a/src/client/debugger/types.ts +++ b/src/client/debugger/types.ts @@ -30,7 +30,6 @@ interface ICommonDebugArguments { debugStdLib?: boolean; logToFile?: boolean; debugOptions?: DebugOptions[]; - multiProcess?: boolean; port?: number; host?: string; // Show return values of functions while stepping. From 7e2794e5991fbc74ddb75050fc3aa8f64967d2f0 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 24 Oct 2018 21:41:26 -0700 Subject: [PATCH 02/12] Fix launch.json --- .vscode/launch.json | 8 +++++--- src/test/index.ts | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 269e1db0336a..347ed5d05464 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -70,8 +70,7 @@ "sourceMaps": true, "outFiles": [ "${workspaceFolder}/out/**/*.js" - ], - "preLaunchTask": "Compile" + ] }, { "name": "Tests (Multiroot, VS Code, *.test.ts)", @@ -126,7 +125,10 @@ "sourceMaps": true, "outFiles": [ "${workspaceFolder}/out/**/*.js" - ] + ], + "env": { + "MOCHA_REPORTER_JUNIT": "true" + } }, { "name": "Unit Tests (without VS Code, *.unit.test.ts)", diff --git a/src/test/index.ts b/src/test/index.ts index 8482ddc3f149..985139d420eb 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -61,5 +61,5 @@ process.on('unhandledRejection', (ex: string | Error, a) => { console.error(`Unhandled Promise Rejection with the message ${message.join(', ')}`); }); -testRunner.configure(options, { coverageConfig: '../build/coverconfig.json' }); +testRunner.configure(options, { coverageConfig: '../coverconfig.json' }); module.exports = testRunner; From 1c504ab5f0e56de5ca5eca4d18a5233efe01a2db Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 09:01:40 -0700 Subject: [PATCH 03/12] Stop tracking and killing of debug processes --- gulpfile.js | 6 +- .../hooks/processTerminationHandler.ts | 103 ----- .../hooks/processTerminationService.ts | 96 ----- src/client/debugger/extension/hooks/types.ts | 10 +- .../debugger/extension/serviceRegistry.ts | 6 +- .../processTerminationHandler.unit.test.ts | 388 ------------------ .../hooks/processTerminationService.test.ts | 109 ----- 7 files changed, 6 insertions(+), 712 deletions(-) delete mode 100644 src/client/debugger/extension/hooks/processTerminationHandler.ts delete mode 100644 src/client/debugger/extension/hooks/processTerminationService.ts delete mode 100644 src/test/debugger/extension/hooks/processTerminationHandler.unit.test.ts delete mode 100644 src/test/debugger/extension/hooks/processTerminationService.test.ts diff --git a/gulpfile.js b/gulpfile.js index 317c2081da78..6be9741cc62d 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -562,11 +562,13 @@ function getFileListToProcess(options) { // If we need only modified files, then filter the glob. if (options && options.mode === 'changes') { - return getModifiedFilesSync(); + return getModifiedFilesSync() + .filter(file => fs.existsSync(file)); } if (options && options.mode === 'staged') { - return getStagedFilesSync(); + return getStagedFilesSync() + .filter(file => fs.existsSync(file)); } return all; diff --git a/src/client/debugger/extension/hooks/processTerminationHandler.ts b/src/client/debugger/extension/hooks/processTerminationHandler.ts deleted file mode 100644 index 7c219bc2688a..000000000000 --- a/src/client/debugger/extension/hooks/processTerminationHandler.ts +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import * as _ from 'lodash'; -import { DebugSession, DebugSessionCustomEvent } from 'vscode'; -import { DebugProtocol } from 'vscode-debugprotocol'; -import { sleep } from '../../../common/utils/async'; -import { swallowExceptions } from '../../../common/utils/decorators'; -import { PTVSDEvents } from './constants'; -import { ChildProcessLaunchData, IDebugSessionEventHandlers, IProcessTerminationService } from './types'; - -/** - * This class is responsible for handling spawning of new processes for debugging and termination of debugger. - * We need to kill off any child processes belonging to the parent process that was debugged via a launch. - * @export - * @class ProcessTerminationEventHandler - * @implements {IDebugSessionEventHandlers} - * @implements {Disposable} - */ -@injectable() -export class ProcessTerminationEventHandler implements IDebugSessionEventHandlers { - /** - * List of PID that need to be killed when debugging ends. - * @protected - * @memberof ProcessTerminationEventHandler - */ - protected procecssIdsTrackedToKill = new Set(); - /** - * Key value pair of the Debug Session ID + corresponding PID that need to be killed. - * @protected - * @memberof ProcessTerminationEventHandler - */ - protected debugSessionsTrackedToKill = new Map(); - constructor(@inject(IProcessTerminationService) private readonly processTermination: IProcessTerminationService) { } - - @swallowExceptions('Track processes for termination') - public async handleCustomEvent(event: DebugSessionCustomEvent): Promise { - if (!event) { - return; - } - - switch (event.event) { - case PTVSDEvents.ChildProcessLaunched: - return this.handleSubProcessLaunch(event.body! as ChildProcessLaunchData); - case PTVSDEvents.ProcessLaunched: - // tslint:disable-next-line:no-any - return this.handleProcessLaunch(event as any as DebugProtocol.ProcessEvent, event.session.id); - default: - return; - } - } - @swallowExceptions('Terminate debugger processes') - public async handleTerminateEvent(event: DebugSession): Promise { - const pid = this.debugSessionsTrackedToKill.get(event.id); - if (pid) { - this.processTermination.terminateProcess(pid); - } - await this.waitForCleanup(); - this.processTermination.terminateOrphanedProcesses(); - } - protected async waitForCleanup() { - // Wait till all house cleaning to take place. - await sleep(5000); - } - protected handleProcessLaunch(event: DebugProtocol.ProcessEvent, debugSessionId: string) { - if (!event.body.systemProcessId) { - return; - } - - switch (event.body.startMethod) { - case 'launch': { - this.processTermination.trackProcess(event.body.systemProcessId); - this.debugSessionsTrackedToKill.set(debugSessionId, event.body.systemProcessId); - break; - } - case 'attach': { - // Only if attaching to a child process part of a multi process launch debug. - if (this.procecssIdsTrackedToKill.has(event.body.systemProcessId)) { - this.procecssIdsTrackedToKill.delete(event.body.systemProcessId); - this.debugSessionsTrackedToKill.set(debugSessionId, event.body.systemProcessId); - } - } - default: - break; - } - } - protected handleSubProcessLaunch(data: ChildProcessLaunchData) { - // We need to track root & parent process that is a part of multi-proc `launch` debugging. - if (!data.rootProcessId || data.rootStartRequest.arguments.request !== 'launch') { - return; - } - this.processTermination.trackProcess(data.parentProcessId); - // this.processTermination.trackProcess(data.processId, data.parentProcessId); - // this.processTermination.trackProcess(data.processId, data.rootProcessId); - - this.procecssIdsTrackedToKill.add(data.processId); - this.procecssIdsTrackedToKill.add(data.parentProcessId); - this.procecssIdsTrackedToKill.add(data.rootProcessId); - } -} diff --git a/src/client/debugger/extension/hooks/processTerminationService.ts b/src/client/debugger/extension/hooks/processTerminationService.ts deleted file mode 100644 index d4262e2c9e3e..000000000000 --- a/src/client/debugger/extension/hooks/processTerminationService.ts +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import * as _ from 'lodash'; -import { Disposable } from 'vscode'; -import { IDebugService } from '../../../common/application/types'; -import { ProcessService } from '../../../common/process/proc'; -import { IDisposableRegistry } from '../../../common/types'; -import { IProcessTerminationService } from './types'; -/** - * Keeps track of processes and child proceses that are being debugged (via a launc), which - * will need to be killed off when the debugger stops. - * @export - * @class ProcessTerminationEventHandler - * @implements {ProcessTerminationService} - * @implements {Disposable} - */ -@injectable() -export class ProcessTerminationService implements IProcessTerminationService { - protected parentAndChildProcsToKill = new Map>(); - protected initialized = false; - constructor(@inject(IDisposableRegistry) disposables: Disposable[]) { - disposables.push(this); - } - public dispose = () => this.terminateTrackedProcesses(); - public trackProcess(pid: number, ancestorPid?: number): void { - // If we have an ancestor Pid, then track those as well, they'll need to be killed off, - // and we'll need to kill the child procs when the ancestors die. - if (ancestorPid) { - if (this.parentAndChildProcsToKill.has(ancestorPid)) { - this.parentAndChildProcsToKill.get(ancestorPid)!.add(pid); - } else { - this.parentAndChildProcsToKill.set(ancestorPid, new Set([ancestorPid, pid])); - } - } - // Track the proc that needs to be killed off (including any of its children). - if (!this.parentAndChildProcsToKill.has(pid)) { - this.parentAndChildProcsToKill.set(pid, new Set([pid])); - } - } - public terminateProcess(pid: number): void { - ProcessService.kill(pid); - this.terminateProcesses(pid); - } - public terminateTrackedProcesses(): void { - for (const kv of this.parentAndChildProcsToKill) { - for (const childProcIds of kv[1].values()) { - try { - ProcessService.kill(childProcIds); - } catch { - // Ignore. - } - } - } - this.parentAndChildProcsToKill.clear(); - } - public terminateOrphanedProcesses() { - this.terminateProcesses(); - } - - public terminateProcesses(parentPid?: number) { - - const parentProcId = parentPid ? [parentPid] : []; - const procIds = [...this.getDeadProcessIds(), ...parentProcId]; - const childProcIds = _.flatten(procIds.map(item => this.getAllChildProcs(item))); - // Kill the parent and all tracked child processes. - [...procIds, ...childProcIds].forEach(procId => { - try { - this.parentAndChildProcsToKill.delete(procId); - ProcessService.kill(procId); - } catch { - // Ignore. - } - }); - } - - protected getDeadProcessIds() { - return Array.from(this.parentAndChildProcsToKill.keys()) - .filter(pid => !ProcessService.isAlive(pid)); - } - protected getAllChildProcs(parentPid: number) { - const childProcIds: number[] = []; - for (const kv of this.parentAndChildProcsToKill) { - if (kv[0] !== parentPid) { - continue; - } - const values = Array.from(kv[1].values()); - childProcIds.push(...values.filter(pid => pid !== parentPid)); - } - const grandChildren = _.flatten(childProcIds.map(pid => this.getAllChildProcs(pid))); - return [...childProcIds, ...grandChildren]; - } -} diff --git a/src/client/debugger/extension/hooks/types.ts b/src/client/debugger/extension/hooks/types.ts index 8228848f2d2d..b62ae91b2998 100644 --- a/src/client/debugger/extension/hooks/types.ts +++ b/src/client/debugger/extension/hooks/types.ts @@ -3,7 +3,7 @@ 'use strict'; -import { DebugSession, DebugSessionCustomEvent, Disposable } from 'vscode'; +import { DebugSession, DebugSessionCustomEvent } from 'vscode'; import { AttachRequestArguments, LaunchRequestArguments } from '../../types'; export const IDebugSessionEventHandlers = Symbol('IDebugSessionEventHandlers'); @@ -59,11 +59,3 @@ export const IChildProcessAttachService = Symbol('IChildProcessAttachService'); export interface IChildProcessAttachService { attach(data: ChildProcessLaunchData): Promise; } - -export const IProcessTerminationService = Symbol('IProcessTerminationService'); -export interface IProcessTerminationService extends Disposable { - trackProcess(pid: number, parentOrGrandParentPid?: number): void; - terminateProcess(pid: number): void; - terminateOrphanedProcesses(): void; - terminateTrackedProcesses(): void; -} diff --git a/src/client/debugger/extension/serviceRegistry.ts b/src/client/debugger/extension/serviceRegistry.ts index cbf86d9273b3..7d669c53ff70 100644 --- a/src/client/debugger/extension/serviceRegistry.ts +++ b/src/client/debugger/extension/serviceRegistry.ts @@ -11,17 +11,13 @@ import { PythonV2DebugConfigurationProvider } from './configProviders/pythonV2Pr import { IConfigurationProviderUtils } from './configProviders/types'; import { ChildProcessAttachEventHandler } from './hooks/childProcessAttachHandler'; import { ChildProcessAttachService } from './hooks/childProcessAttachService'; -import { ProcessTerminationEventHandler } from './hooks/processTerminationHandler'; -import { ProcessTerminationService } from './hooks/processTerminationService'; -import { IChildProcessAttachService, IDebugSessionEventHandlers, IProcessTerminationService } from './hooks/types'; +import { IChildProcessAttachService, IDebugSessionEventHandlers } from './hooks/types'; import { IDebugConfigurationProvider, IDebuggerBanner } from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IDebugConfigurationProvider, PythonV2DebugConfigurationProvider); serviceManager.addSingleton(IConfigurationProviderUtils, ConfigurationProviderUtils); serviceManager.addSingleton(IDebuggerBanner, DebuggerBanner); - serviceManager.addSingleton(IProcessTerminationService, ProcessTerminationService); serviceManager.addSingleton(IChildProcessAttachService, ChildProcessAttachService); serviceManager.addSingleton(IDebugSessionEventHandlers, ChildProcessAttachEventHandler); - serviceManager.addSingleton(IDebugSessionEventHandlers, ProcessTerminationEventHandler); } diff --git a/src/test/debugger/extension/hooks/processTerminationHandler.unit.test.ts b/src/test/debugger/extension/hooks/processTerminationHandler.unit.test.ts deleted file mode 100644 index 325f3eceacdf..000000000000 --- a/src/test/debugger/extension/hooks/processTerminationHandler.unit.test.ts +++ /dev/null @@ -1,388 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -// tslint:disable:no-any max-func-body-length max-classes-per-file - -import { anything, instance, mock, verify, when } from 'ts-mockito'; -import { DebugSession, DebugSessionCustomEvent, Disposable } from 'vscode'; -import { DebugProtocol } from 'vscode-debugprotocol'; -import { noop } from '../../../../client/common/utils/misc'; -import { PTVSDEvents } from '../../../../client/debugger/extension/hooks/constants'; -import { ProcessTerminationEventHandler } from '../../../../client/debugger/extension/hooks/processTerminationHandler'; -import { ProcessTerminationService } from '../../../../client/debugger/extension/hooks/processTerminationService'; -import { ChildProcessLaunchData } from '../../../../client/debugger/extension/hooks/types'; -import { AttachRequestArguments, LaunchRequestArguments } from '../../../../client/debugger/types'; - -suite('Debugy - Process Termination Handler', () => { - test('Exceptions are not bubbled up if exceptions are thrown', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new ProcessTerminationEventHandler(instance(procTerminationService)); - const event: DebugProtocol.ProcessEvent = { - event: PTVSDEvents.ProcessLaunched, - type: '', - seq: 1, - body: { - systemProcessId: 1, - name: '', - startMethod: 'launch' - } - } as any; - const session = { - id: '1234' - }; - (event as any).session = session; - - when(procTerminationService.trackProcess(event.body.systemProcessId)).thenThrow(new Error('Kaboom')); - await handler.handleCustomEvent(event as any); - verify(procTerminationService.trackProcess(event.body.systemProcessId)).once(); - }); - test('Track child processes where main process was launched', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new ProcessTerminationEventHandler(instance(procTerminationService)); - - const args: LaunchRequestArguments | AttachRequestArguments = { - request: 'launch', - type: 'python', - name: '' - }; - const body: ChildProcessLaunchData = { - rootProcessId: 1, - parentProcessId: 1, - port: 1234, - processId: 2, - rootStartRequest: { - seq: 1, - type: 'python', - arguments: args, - command: 'request' - } - }; - const event: DebugSessionCustomEvent = { - event: PTVSDEvents.ChildProcessLaunched, - session: {} as any, - body - }; - await handler.handleCustomEvent(event as any); - verify(procTerminationService.trackProcess(body.processId, body.parentProcessId)).twice(); - }); - test('Track child processes where main process was launched', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new ProcessTerminationEventHandler(instance(procTerminationService)); - - const args: LaunchRequestArguments | AttachRequestArguments = { - request: 'launch', - type: 'python', - name: '' - }; - const body: ChildProcessLaunchData = { - rootProcessId: 1, - parentProcessId: 1, - port: 1234, - processId: 2, - rootStartRequest: { - seq: 1, - type: 'python', - arguments: args, - command: 'request' - } - }; - const event: DebugSessionCustomEvent = { - event: PTVSDEvents.ChildProcessLaunched, - session: {} as any, - body - }; - await handler.handleCustomEvent(event as any); - verify(procTerminationService.trackProcess(body.processId, body.parentProcessId)).atLeast(1); - }); - test('Do not Track child processes where main process was attached to', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new ProcessTerminationEventHandler(instance(procTerminationService)); - - const args: LaunchRequestArguments | AttachRequestArguments = { - request: 'attach', - type: 'python', - name: '' - }; - const body: ChildProcessLaunchData = { - rootProcessId: 1, - parentProcessId: 1, - port: 1234, - processId: 2, - rootStartRequest: { - seq: 1, - type: 'python', - arguments: args, - command: 'request' - } - }; - const event: DebugSessionCustomEvent = { - event: PTVSDEvents.ChildProcessLaunched, - session: {} as any, - body - }; - await handler.handleCustomEvent(event as any); - verify(procTerminationService.trackProcess(anything(), anything())).never(); - }); - test('Track child processes where main process was launched, and parent and root differ', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new ProcessTerminationEventHandler(instance(procTerminationService)); - - const args: LaunchRequestArguments | AttachRequestArguments = { - request: 'launch', - type: 'python', - name: '' - }; - const body: ChildProcessLaunchData = { - rootProcessId: 1, - parentProcessId: 2, - port: 1234, - processId: 3, - rootStartRequest: { - seq: 1, - type: 'python', - arguments: args, - command: 'request' - } - }; - const event: DebugSessionCustomEvent = { - event: PTVSDEvents.ChildProcessLaunched, - session: {} as any, - body - }; - await handler.handleCustomEvent(event as any); - verify(procTerminationService.trackProcess(body.processId, body.parentProcessId)).once(); - verify(procTerminationService.trackProcess(body.processId, body.rootProcessId)).once(); - }); - test('Track processes launched', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new ProcessTerminationEventHandler(instance(procTerminationService)); - const event: DebugProtocol.ProcessEvent = { - event: PTVSDEvents.ProcessLaunched, - body: { - name: '', - startMethod: 'launch', - systemProcessId: 1 - }, - seq: 1, - type: 'python' - }; - const session = { - id: '1234' - }; - (event as any).session = session; - - await handler.handleCustomEvent(event as any); - verify(procTerminationService.trackProcess(event.body.systemProcessId)).once(); - }); - test('Do not Track processes attached', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new ProcessTerminationEventHandler(instance(procTerminationService)); - const event: DebugProtocol.ProcessEvent = { - event: PTVSDEvents.ProcessLaunched, - body: { - name: '', - startMethod: 'attach', - systemProcessId: 1 - }, - seq: 1, - type: 'python' - }; - await handler.handleCustomEvent(event as any); - verify(procTerminationService.trackProcess(anything())).never(); - }); - test('Handle termination', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new class extends ProcessTerminationEventHandler { - protected async waitForCleanup() { noop(); } - }(instance(procTerminationService)); - await handler.handleTerminateEvent({ name: '', type: '', id: '' } as any as DebugSession); - verify(procTerminationService.terminateOrphanedProcesses()).once(); - }); - test('Handle termination of child process that was attached to as part of multi-proc debugging', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new class extends ProcessTerminationEventHandler { - protected async waitForCleanup() { noop(); } - }(instance(procTerminationService)); - - const childProcessId = 2; - - const args: LaunchRequestArguments | AttachRequestArguments = { - request: 'launch', - type: 'python', - name: '' - }; - const body: ChildProcessLaunchData = { - rootProcessId: 1, - parentProcessId: 1, - port: 1234, - processId: childProcessId, - rootStartRequest: { - seq: 1, - type: 'python', - arguments: args, - command: 'request' - } - }; - const event: DebugSessionCustomEvent = { - event: PTVSDEvents.ChildProcessLaunched, - session: {} as any, - body - }; - - const session: DebugSession = { - name: '', - type: '', - id: '9989098080' - } as any; - const processEvent: DebugProtocol.ProcessEvent = { - body: { - name: '', - systemProcessId: childProcessId, - startMethod: 'attach' - }, - event: PTVSDEvents.ProcessLaunched, - seq: 1, - type: '' - }; - (processEvent as any).session = session; - - // Handle event to attach to child process. - await handler.handleCustomEvent(event); - // Handle event for when `process` event has been created for above child process. - await handler.handleCustomEvent(processEvent as any as DebugSessionCustomEvent); - // Handle event for when debug session related to child process has terminated. - await handler.handleTerminateEvent(session); - - verify(procTerminationService.terminateOrphanedProcesses()).once(); - verify(procTerminationService.terminateProcess(body.processId)).once(); - }); - test('Ensure child process does not terminate when parent process was attached to', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new class extends ProcessTerminationEventHandler { - protected async waitForCleanup() { noop(); } - }(instance(procTerminationService)); - - const childProcessId = 2; - - const args: LaunchRequestArguments | AttachRequestArguments = { - request: 'attach', - type: 'python', - name: '' - }; - const body: ChildProcessLaunchData = { - rootProcessId: 1, - parentProcessId: 1, - port: 1234, - processId: childProcessId, - rootStartRequest: { - seq: 1, - type: 'python', - arguments: args, - command: 'request' - } - }; - const event: DebugSessionCustomEvent = { - event: PTVSDEvents.ChildProcessLaunched, - session: {} as any, - body - }; - - const session: DebugSession = { - name: '', - type: '', - id: '9989098080' - } as any; - const processEvent: DebugProtocol.ProcessEvent = { - body: { - name: '', - systemProcessId: childProcessId, - startMethod: 'attach' - }, - event: PTVSDEvents.ProcessLaunched, - seq: 1, - type: '' - }; - (processEvent as any).session = session; - - // Handle event to attach to child process. - await handler.handleCustomEvent(event); - // Handle event for when `process` event has been created for above child process. - await handler.handleCustomEvent(processEvent as any as DebugSessionCustomEvent); - // Handle event for when debug session related to child process has terminated. - await handler.handleTerminateEvent(session); - - verify(procTerminationService.terminateOrphanedProcesses()).once(); - verify(procTerminationService.terminateProcess(body.processId)).never(); - }); - test('Handle termination of process that was launched', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new class extends ProcessTerminationEventHandler { - protected async waitForCleanup() { noop(); } - }(instance(procTerminationService)); - - const processId = 2; - const session: DebugSession = { - name: '', - type: '', - id: '9989098080' - } as any; - const processEvent: DebugProtocol.ProcessEvent = { - body: { - name: '', - systemProcessId: processId, - startMethod: 'launch' - }, - event: PTVSDEvents.ProcessLaunched, - seq: 1, - type: '' - }; - (processEvent as any).session = session; - - // Handle event for when process created via a launch of a debugger. - await handler.handleCustomEvent(processEvent as any as DebugSessionCustomEvent); - // Handle event for when debug session related to above process has terminated. - await handler.handleTerminateEvent(session); - - verify(procTerminationService.terminateOrphanedProcesses()).once(); - verify(procTerminationService.terminateProcess(processId)).once(); - }); - test('Ensure process launched is not terminated when another debugger ends', async () => { - const procTerminationService = mock(ProcessTerminationService); - const handler = new class extends ProcessTerminationEventHandler { - protected async waitForCleanup() { noop(); } - }(instance(procTerminationService)); - - const processId = 2; - const session: DebugSession = { - name: '', - type: '', - id: '9989098080' - } as any; - const processEvent: DebugProtocol.ProcessEvent = { - body: { - name: '', - systemProcessId: processId, - startMethod: 'launch' - }, - event: PTVSDEvents.ProcessLaunched, - seq: 1, - type: '' - }; - (processEvent as any).session = session; - - // Handle event for when process created via a launch of a debugger. - await handler.handleCustomEvent(processEvent as any as DebugSessionCustomEvent); - // Handle event for another debug session terminating. - const anotherSession: DebugSession = { - name: '', - type: '', - id: '1234' - } as any; - await handler.handleTerminateEvent(anotherSession); - - verify(procTerminationService.terminateOrphanedProcesses()).once(); - verify(procTerminationService.terminateProcess(processId)).never(); - }); -}); diff --git a/src/test/debugger/extension/hooks/processTerminationService.test.ts b/src/test/debugger/extension/hooks/processTerminationService.test.ts deleted file mode 100644 index d1db0eb12d9e..000000000000 --- a/src/test/debugger/extension/hooks/processTerminationService.test.ts +++ /dev/null @@ -1,109 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -// tslint:disable:no-any max-func-body-length no-invalid-this max-classes-per-file - -import { expect } from 'chai'; -import { spawn } from 'child_process'; -import { ProcessService } from '../../../../client/common/process/proc'; -import { createDeferred } from '../../../../client/common/utils/async'; -import { ProcessTerminationService } from '../../../../client/debugger/extension/hooks/processTerminationService'; -import { PYTHON_PATH } from '../../../common'; - -suite('Debug - Process Termination', function () { - // tslint:disable-next-line:no-invalid-this - this.timeout(5000); - let procIdsToKill: number[] = []; - teardown(() => { - procIdsToKill.forEach(pid => ProcessService.kill(pid)); - procIdsToKill = []; - }); - - function spawnProc() { - const proc = spawn(PYTHON_PATH, ['-c', 'while(True): import time;time.sleep(0.5);print(1)']); - const exited = createDeferred(); - proc.on('exit', () => exited.resolve(true)); - procIdsToKill.push(proc.pid); - - return { pid: proc.pid, exited: exited.promise }; - } - - test('Orphaned Process is killed', async () => { - const proc = spawnProc(); - const service = new class extends ProcessTerminationService { - protected getDeadProcessIds() { - return [proc.pid]; - } - }([]); - - service.trackProcess(proc.pid); - service.terminateOrphanedProcesses(); - expect(await proc.exited).to.equal(true, 'process did not die'); - }); - test('Process is killed when disposing', async () => { - const proc = spawnProc(); - const service = new class extends ProcessTerminationService { - protected getDeadProcessIds() { - return []; - } - }([]); - - service.trackProcess(proc.pid); - service.dispose(); - expect(await proc.exited).to.equal(true, 'process did not die'); - }); - test('Related child process is killed when parent is killed', async () => { - const parentProc = spawnProc(); - const childProc = spawnProc(); - const service = new class extends ProcessTerminationService { - protected getDeadProcessIds() { - return []; - } - }([]); - - service.trackProcess(childProc.pid, parentProc.pid); - service.terminateProcess(parentProc.pid); - - expect(await parentProc.exited).to.equal(true, 'main process did not die'); - expect(await childProc.exited).to.equal(true, 'child process did not die'); - }); - test('Related child process is killed when grand parent is killed', async () => { - const grandParentProc = spawnProc(); - const parentProc = spawnProc(); - const childProc = spawnProc(); - const service = new class extends ProcessTerminationService { - protected getDeadProcessIds() { - return []; - } - }([]); - - service.trackProcess(parentProc.pid, grandParentProc.pid); - service.trackProcess(childProc.pid, parentProc.pid); - - service.terminateProcess(grandParentProc.pid); - - expect(await grandParentProc.exited).to.equal(true, 'grand parent process did not die'); - expect(await parentProc.exited).to.equal(true, 'main process did not die'); - expect(await childProc.exited).to.equal(true, 'child process did not die'); - }); - test('Related child process is killed when parent is killed, but grand parent is left alive', async () => { - const grandParentProc = spawnProc(); - const parentProc = spawnProc(); - const childProc = spawnProc(); - const service = new class extends ProcessTerminationService { - protected getDeadProcessIds() { - return []; - } - }([]); - - service.trackProcess(parentProc.pid, grandParentProc.pid); - service.trackProcess(childProc.pid, parentProc.pid); - service.terminateProcess(parentProc.pid); - - expect(ProcessService.isAlive(grandParentProc.pid)).to.equal(true, 'grand parent process died'); - expect(await parentProc.exited).to.equal(true, 'main process did not die'); - expect(await childProc.exited).to.equal(true, 'child process did not die'); - }); -}); From d4622b51c91e8e9f8c4d000a781d38ab89ae4070 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 09:08:32 -0700 Subject: [PATCH 04/12] Add pre-launch setting --- .vscode/launch.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 347ed5d05464..00a0b08974b6 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -70,7 +70,8 @@ "sourceMaps": true, "outFiles": [ "${workspaceFolder}/out/**/*.js" - ] + ], + "preLaunchTask": "Compile" }, { "name": "Tests (Multiroot, VS Code, *.test.ts)", From e90bc9fea837f997eec5b87d1a38e42df8649664 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 10:52:16 -0700 Subject: [PATCH 05/12] Update constants --- build/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/constants.ts b/build/constants.ts index 6df1b18d1a49..4b8ba9fb07dd 100644 --- a/build/constants.ts +++ b/build/constants.ts @@ -11,7 +11,7 @@ export const ExtensionRootDir = path.join(__dirname, '..'); const jsonFileWithListOfOldFiles = path.join(__dirname, 'existingFiles.json'); function getListOfExcludedFiles() { const files = JSON.parse(fs.readFileSync(jsonFileWithListOfOldFiles).toString()) as string[]; - return files.map(file => path.join(ExtensionRootDir, file)); + return files.map(file => path.join(ExtensionRootDir, file.replace(/\//g, path.sep))); } export const filesNotToCheck: string[] = getListOfExcludedFiles(); From 4045013b4a0fbbdacd52d01c53f6e3e1441066e8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 10:56:40 -0700 Subject: [PATCH 06/12] Logging files --- gulpfile.js | 1 + 1 file changed, 1 insertion(+) diff --git a/gulpfile.js b/gulpfile.js index 6be9741cc62d..f0abaf822e01 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -529,6 +529,7 @@ function getModifiedFilesSync() { const cmd = `git diff --name-only HEAD ${originOrUpstream}/master`; console.info(cmd); const out = cp.execSync(cmd, { encoding: 'utf8', cwd: __dirname }); + console.log(out); return out .split(/\r?\n/) .filter(l => !!l) From 082ec672fc9af49c49abf9c4e87a0f105873d572 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 11:02:45 -0700 Subject: [PATCH 07/12] Fix paths --- gulpfile.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index f0abaf822e01..7ee20b43d714 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -534,7 +534,7 @@ function getModifiedFilesSync() { .split(/\r?\n/) .filter(l => !!l) .filter(l => l.length > 0) - .map(l => l.trim()) + .map(l => l.trim().replace(/\//g, path.sep)) .map(l => path.join(__dirname, l)); } else { const out = cp.execSync('git status -u -s', { encoding: 'utf8' }); @@ -542,7 +542,7 @@ function getModifiedFilesSync() { .split(/\r?\n/) .filter(l => !!l) .filter(l => _.intersection(['M', 'A', 'R', 'C', 'U', '?'], l.substring(0, 2).trim().split('')).length > 0) - .map(l => path.join(__dirname, l.substring(2).trim())); + .map(l => path.join(__dirname, l.substring(2).trim().replace(/\//g, path.sep))); } } From c5820cf6d23b76343cddc343492b24014158db29 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 11:04:50 -0700 Subject: [PATCH 08/12] Fixes from Rich --- gulpfile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gulpfile.js b/gulpfile.js index 7ee20b43d714..200b8bd388f6 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -84,7 +84,7 @@ const copyrightHeaders = [copyrightHeader.join('\n'), copyrightHeader.join('\r\n gulp.task('precommit', (done) => run({ exitOnError: true, mode: 'staged' }, done)); -gulp.task('hygiene-watch', () => gulp.watch(tsFilter, debounce(() => run({ mode: 'changes', skipFormatCheck: true, skipIndentationCheck: true, skipCopyrightCheck: true }), 100))); +gulp.task('hygiene-watch', () => gulp.watch(tsFilter, gulp.series('hygiene-modified'))); gulp.task('hygiene', (done) => run({ mode: 'all', skipFormatCheck: true, skipIndentationCheck: true }, done)); From fda7587ac45c1d92d896f81136e2e74e97e5c595 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 11:14:04 -0700 Subject: [PATCH 09/12] Update .js --- build/constants.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/constants.js b/build/constants.js index 178ba8f0a006..4f4ec35998d7 100644 --- a/build/constants.js +++ b/build/constants.js @@ -8,6 +8,6 @@ exports.ExtensionRootDir = path.join(__dirname, '..'); const jsonFileWithListOfOldFiles = path.join(__dirname, 'existingFiles.json'); function getListOfExcludedFiles() { const files = JSON.parse(fs.readFileSync(jsonFileWithListOfOldFiles).toString()); - return files.map(file => path.join(exports.ExtensionRootDir, file)); + return files.map(file => path.join(exports.ExtensionRootDir, file.replace(/\//g, path.sep))); } exports.filesNotToCheck = getListOfExcludedFiles(); From 5d06fd386fec51fee502484bdad5e3fbad40e87f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 11:23:41 -0700 Subject: [PATCH 10/12] logging --- build/tslint-rules/baseRuleWalker.js | 3 +++ build/tslint-rules/copyrightAndStrictHeaderRule.js | 2 ++ 2 files changed, 5 insertions(+) diff --git a/build/tslint-rules/baseRuleWalker.js b/build/tslint-rules/baseRuleWalker.js index 057ba97fe0aa..301e8eb404f8 100644 --- a/build/tslint-rules/baseRuleWalker.js +++ b/build/tslint-rules/baseRuleWalker.js @@ -11,6 +11,9 @@ class BaseRuleWalker extends Lint.RuleWalker { } sholdIgnoreCcurrentFile(node) { const sourceFile = node.getSourceFile(); + if (sourceFile && sourceFile.fileName) { + console.log(sourceFile.fileName); + } return sourceFile && sourceFile.fileName && this.filesToIgnore.indexOf(sourceFile.fileName) >= 0; } } diff --git a/build/tslint-rules/copyrightAndStrictHeaderRule.js b/build/tslint-rules/copyrightAndStrictHeaderRule.js index 4c14dde4e74a..213311fbdebf 100644 --- a/build/tslint-rules/copyrightAndStrictHeaderRule.js +++ b/build/tslint-rules/copyrightAndStrictHeaderRule.js @@ -29,6 +29,8 @@ class NoFileWithoutCopyrightHeader extends baseRuleWalker_1.BaseRuleWalker { return; } } + console.log(_sourceFile.fileName); + console.log(this.sholdIgnoreCcurrentFile(_sourceFile)); const line1 = sourceFileContents.length > 0 ? sourceFileContents.split(/\r\n|\r|\n/)[0] : ''; const fix = Lint.Replacement.appendText(0, `${copyrightHeader.join(os_1.EOL)}\n\n`); this.addFailure(this.createFailure(0, line1.length, failureMessage, fix)); From c4342c5085e6f59885d064f748b5418206013972 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 11:24:07 -0700 Subject: [PATCH 11/12] logging --- build/constants.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build/constants.js b/build/constants.js index 4f4ec35998d7..d44371b56086 100644 --- a/build/constants.js +++ b/build/constants.js @@ -8,6 +8,8 @@ exports.ExtensionRootDir = path.join(__dirname, '..'); const jsonFileWithListOfOldFiles = path.join(__dirname, 'existingFiles.json'); function getListOfExcludedFiles() { const files = JSON.parse(fs.readFileSync(jsonFileWithListOfOldFiles).toString()); - return files.map(file => path.join(exports.ExtensionRootDir, file.replace(/\//g, path.sep))); + const x = files.map(file => path.join(exports.ExtensionRootDir, file.replace(/\//g, path.sep))); + console.log(x); + return x; } exports.filesNotToCheck = getListOfExcludedFiles(); From 423fa818ecc1e546a012ef592008ef74a5e56888 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 25 Oct 2018 11:32:15 -0700 Subject: [PATCH 12/12] Fixes --- build/constants.js | 4 +--- build/tslint-rules/baseRuleWalker.js | 6 ++---- build/tslint-rules/baseRuleWalker.ts | 3 ++- build/tslint-rules/copyrightAndStrictHeaderRule.js | 2 -- build/tslint-rules/messagesMustBeLocalizedRule.js | 2 +- gulpfile.js | 1 - 6 files changed, 6 insertions(+), 12 deletions(-) diff --git a/build/constants.js b/build/constants.js index d44371b56086..4f4ec35998d7 100644 --- a/build/constants.js +++ b/build/constants.js @@ -8,8 +8,6 @@ exports.ExtensionRootDir = path.join(__dirname, '..'); const jsonFileWithListOfOldFiles = path.join(__dirname, 'existingFiles.json'); function getListOfExcludedFiles() { const files = JSON.parse(fs.readFileSync(jsonFileWithListOfOldFiles).toString()); - const x = files.map(file => path.join(exports.ExtensionRootDir, file.replace(/\//g, path.sep))); - console.log(x); - return x; + return files.map(file => path.join(exports.ExtensionRootDir, file.replace(/\//g, path.sep))); } exports.filesNotToCheck = getListOfExcludedFiles(); diff --git a/build/tslint-rules/baseRuleWalker.js b/build/tslint-rules/baseRuleWalker.js index 301e8eb404f8..8104718af1e1 100644 --- a/build/tslint-rules/baseRuleWalker.js +++ b/build/tslint-rules/baseRuleWalker.js @@ -2,6 +2,7 @@ // Licensed under the MIT License. 'use strict'; Object.defineProperty(exports, "__esModule", { value: true }); +const path = require("path"); const Lint = require("tslint"); const constants_1 = require("../constants"); class BaseRuleWalker extends Lint.RuleWalker { @@ -11,10 +12,7 @@ class BaseRuleWalker extends Lint.RuleWalker { } sholdIgnoreCcurrentFile(node) { const sourceFile = node.getSourceFile(); - if (sourceFile && sourceFile.fileName) { - console.log(sourceFile.fileName); - } - return sourceFile && sourceFile.fileName && this.filesToIgnore.indexOf(sourceFile.fileName) >= 0; + return sourceFile && sourceFile.fileName && this.filesToIgnore.indexOf(sourceFile.fileName.replace(/\//g, path.sep)) >= 0; } } exports.BaseRuleWalker = BaseRuleWalker; diff --git a/build/tslint-rules/baseRuleWalker.ts b/build/tslint-rules/baseRuleWalker.ts index 28097c0ceceb..e30d0e7c2081 100644 --- a/build/tslint-rules/baseRuleWalker.ts +++ b/build/tslint-rules/baseRuleWalker.ts @@ -3,6 +3,7 @@ 'use strict'; +import * as path from 'path'; import * as Lint from 'tslint'; import * as ts from 'typescript'; import { filesNotToCheck } from '../constants'; @@ -11,6 +12,6 @@ export class BaseRuleWalker extends Lint.RuleWalker { private readonly filesToIgnore = filesNotToCheck; protected sholdIgnoreCcurrentFile(node: ts.Node) { const sourceFile = node.getSourceFile(); - return sourceFile && sourceFile.fileName && this.filesToIgnore.indexOf(sourceFile.fileName) >= 0; + return sourceFile && sourceFile.fileName && this.filesToIgnore.indexOf(sourceFile.fileName.replace(/\//g, path.sep)) >= 0; } } diff --git a/build/tslint-rules/copyrightAndStrictHeaderRule.js b/build/tslint-rules/copyrightAndStrictHeaderRule.js index 213311fbdebf..4c14dde4e74a 100644 --- a/build/tslint-rules/copyrightAndStrictHeaderRule.js +++ b/build/tslint-rules/copyrightAndStrictHeaderRule.js @@ -29,8 +29,6 @@ class NoFileWithoutCopyrightHeader extends baseRuleWalker_1.BaseRuleWalker { return; } } - console.log(_sourceFile.fileName); - console.log(this.sholdIgnoreCcurrentFile(_sourceFile)); const line1 = sourceFileContents.length > 0 ? sourceFileContents.split(/\r\n|\r|\n/)[0] : ''; const fix = Lint.Replacement.appendText(0, `${copyrightHeader.join(os_1.EOL)}\n\n`); this.addFailure(this.createFailure(0, line1.length, failureMessage, fix)); diff --git a/build/tslint-rules/messagesMustBeLocalizedRule.js b/build/tslint-rules/messagesMustBeLocalizedRule.js index f7b7616a31f9..8e9c7b6a0163 100644 --- a/build/tslint-rules/messagesMustBeLocalizedRule.js +++ b/build/tslint-rules/messagesMustBeLocalizedRule.js @@ -12,7 +12,7 @@ const methodNames = [ // From IOutputChannel (vscode.OutputChannel) 'appendLine', 'appendLine' ]; -const failureMessage = 'Messages must be localized in the Python Extension'; +const failureMessage = 'Messages must be localized in the Python Extension (use src/client/common/utils/localize.ts)'; class NoStringLiteralsInMessages extends baseRuleWalker_1.BaseRuleWalker { visitCallExpression(node) { const prop = node.expression; diff --git a/gulpfile.js b/gulpfile.js index 200b8bd388f6..084d1e175861 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -529,7 +529,6 @@ function getModifiedFilesSync() { const cmd = `git diff --name-only HEAD ${originOrUpstream}/master`; console.info(cmd); const out = cp.execSync(cmd, { encoding: 'utf8', cwd: __dirname }); - console.log(out); return out .split(/\r?\n/) .filter(l => !!l)