Terminate child processes launched and attached to in multi proc debu…

…gging (#2954)

* Multi proc debugging
* Terminate child procs when terminating multiproc debugging
* Change default flask debug configs
* Fix typo
* None of the debugger tests are allowed to fail
* Fix typos
* Incremental changes
* Added tests
* Telemetry
* Allow release PTVSD tests to fail
* Add --client flag

.travis.yml

@@ -43,21 +43,12 @@ matrix:
           python: "3.7-dev"
           env: MULTIROOT_WORKSPACE_TEST=true
     allow_failures:
-        - os: linux
-          python: "2.7"
-          env: DEBUGGER_TEST=true
         - os: linux
           python: "2.7"
           env: DEBUGGER_TEST_RELEASE=true
-        - os: linux
-          python: "3.6-dev"
-          env: DEBUGGER_TEST=true
         - os: linux
           python: "3.6-dev"
           env: DEBUGGER_TEST_RELEASE=true
-        - os: linux
-          python: "3.7-dev"
-          env: DEBUGGER_TEST=true
         - os: linux
           python: "3.7-dev"
           env: DEBUGGER_TEST_RELEASE=true

news/1 Enhancements/2326.md

@@ -0,0 +1 @@
+Add support for automatic reloading of flask in the debugger.

news/1 Enhancements/80.md

@@ -0,0 +1 @@
+Add support for multi process debugging.

package.json

@@ -361,12 +361,10 @@
                             "env": {
                                 "FLASK_APP": "app.py",
                                 "FLASK_ENV": "development",
-                                "FLASK_DEBUG": "0"
+                                "FLASK_DEBUG": "1"
                             },
                             "args": [
-                                "run",
-                                "--no-debugger",
-                                "--no-reload"
+                                "run"
                             ],
                             "jinja": true
                         }
@@ -688,12 +686,12 @@
                         "request": "launch",
                         "module": "flask",
                         "env": {
-                            "FLASK_APP": "app.py"
-                        },
+                            "FLASK_APP": "app.py",
+                            "FLASK_ENV": "development",
+                            "FLASK_DEBUG": "1"
+                    },
                         "args": [
-                            "run",
-                            "--no-debugger",
-                            "--no-reload"
+                            "run"
                         ],
                         "jinja": true
                     },

src/client/common/process/proc.ts

@@ -13,6 +13,24 @@ import { ExecutionResult, IBufferDecoder, IProcessService, ObservableExecutionRe
 
 export class ProcessService implements IProcessService {
     constructor(private readonly decoder: IBufferDecoder, private readonly env?: EnvironmentVariables) { }
+    public static isAlive(pid: number): boolean {
+        try {
+            process.kill(pid, 0);
+            return true;
+        } catch {
+            return false;
+        }
+    }
+    public static kill(pid: number): void {
+        // tslint:disable-next-line:no-require-imports
+        const killProcessTree = require('tree-kill');
+        try {
+            killProcessTree(pid);
+        } catch {
+            // Ignore.
+        }
+
+    }
     public execObservable(file: string, args: string[], options: SpawnOptions = {}): ObservableExecutionResult<string> {
         const encoding = options.encoding = typeof options.encoding === 'string' && options.encoding.length > 0 ? options.encoding : DEFAULT_ENCODING;
         delete options.encoding;

src/client/debugger/debugAdapter/Common/Contracts.ts

@@ -3,27 +3,6 @@
 
 'use strict';
 
-// tslint:disable:interface-name member-access no-single-line-block-comment no-any no-stateless-class member-ordering prefer-method-signature no-unnecessary-class
-
-import { OutputEvent } from 'vscode-debugadapter';
-import { DebuggerPerformanceTelemetry, DebuggerTelemetry } from '../../../telemetry/types';
-
-export class TelemetryEvent extends OutputEvent {
-    body!: {
-        /** The category of output (such as: 'console', 'stdout', 'stderr', 'telemetry'). If not specified, 'console' is assumed. */
-        category: string;
-        /** The output to report. */
-        output: string;
-        /** Optional data to report. For the 'telemetry' category the data will be sent to telemetry, for the other categories the data is shown in JSON format. */
-        data?: any;
-    };
-    constructor(output: string, data?: DebuggerTelemetry | DebuggerPerformanceTelemetry) {
-        super(output, 'telemetry');
-        if (data) {
-            this.body.data = data;
-        }
-    }
-}
 export interface IDebugServer {
     port: number;
     host?: string;

src/client/debugger/debugAdapter/DebugClients/localDebugClientV2.ts

@@ -4,7 +4,7 @@
 'use strict';
 
 import { DebugSession } from 'vscode-debugadapter';
-import { LaunchRequestArguments } from '../../types';
+import { IKnownLaunchRequestArguments, LaunchRequestArguments } from '../../types';
 import { IDebugLauncherScriptProvider } from '../types';
 import { LocalDebugClient } from './LocalDebugClient';
 
@@ -14,8 +14,16 @@ export class LocalDebugClientV2 extends LocalDebugClient {
     }
     protected buildDebugArguments(cwd: string, debugPort: number): string[] {
         const launcher = this.launcherScriptProvider.getLauncherFilePath();
-        const noDebugArg = this.args.noDebug ? ['--nodebug'] : [];
-        return [launcher, ...noDebugArg, '--host', 'localhost', '--port', debugPort.toString()];
+        const additionalPtvsdArgs: string[] = [];
+        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() {
         const programArgs = Array.isArray(this.args.args) && this.args.args.length > 0 ? this.args.args : [];

src/client/debugger/extension/configProviders/pythonV2Provider.ts

@@ -9,7 +9,7 @@ import { IPlatformService } from '../../../common/platform/types';
 import { IServiceContainer } from '../../../ioc/types';
 import { sendTelemetryEvent } from '../../../telemetry';
 import { DEBUGGER } from '../../../telemetry/constants';
-import { DebuggerTelemetryV2 } from '../../../telemetry/types';
+import { DebuggerTelemetry } from '../../../telemetry/types';
 import { AttachRequestArguments, DebugOptions, LaunchRequestArguments } from '../../types';
 import { BaseConfigurationProvider } from './baseProvider';
 import { IConfigurationProviderUtils } from './types';
@@ -127,7 +127,7 @@ export class PythonV2DebugConfigurationProvider extends BaseConfigurationProvide
         return (debugConfiguration.module && debugConfiguration.module.toUpperCase() === 'FLASK') ? true : false;
     }
     private sendTelemetry(trigger: 'launch' | 'attach', debugConfiguration: Partial<LaunchRequestArguments & AttachRequestArguments>) {
-        const telemetryProps: DebuggerTelemetryV2 = {
+        const telemetryProps: DebuggerTelemetry = {
             trigger,
             console: debugConfiguration.console,
             hasEnvVars: typeof debugConfiguration.env === 'object' && Object.keys(debugConfiguration.env).length > 0,

src/client/debugger/extension/hooks/childProcessAttachHandler.ts

@@ -0,0 +1,31 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+import { inject, injectable } from 'inversify';
+import { DebugSessionCustomEvent } from 'vscode';
+import { swallowExceptions } from '../../../common/utils/decorators';
+import { PTVSDEvents } from './constants';
+import { ChildProcessLaunchData, IChildProcessAttachService, IDebugSessionEventHandlers } from './types';
+
+/**
+ * This class is responsible for automatically attaching the debugger to any
+ * child processes launched. I.e. this is the classs responsible for multi-proc debugging.
+ * @export
+ * @class ChildProcessAttachEventHandler
+ * @implements {IDebugSessionEventHandlers}
+ */
+@injectable()
+export class ChildProcessAttachEventHandler implements IDebugSessionEventHandlers {
+    constructor(@inject(IChildProcessAttachService) private readonly childProcessAttachService: IChildProcessAttachService) { }
+
+    @swallowExceptions('Handle child process launch')
+    public async handleCustomEvent(event: DebugSessionCustomEvent): Promise<void> {
+        if (!event || event.event !== PTVSDEvents.ChildProcessLaunched) {
+            return;
+        }
+        const data = event.body! as ChildProcessLaunchData;
+        await this.childProcessAttachService.attach(data);
+    }
+}

src/client/debugger/extension/hooks/childProcessLaunchHandler.ts → src/client/debugger/extension/hooks/childProcessAttachService.ts

@@ -4,42 +4,29 @@
 'use strict';
 
 import { inject, injectable } from 'inversify';
-import { DebugConfiguration, DebugSessionCustomEvent, WorkspaceFolder } from 'vscode';
+import { DebugConfiguration, WorkspaceFolder } from 'vscode';
 import { IApplicationShell, IDebugService, IWorkspaceService } from '../../../common/application/types';
-import { swallowExceptions } from '../../../common/utils/decorators';
 import { noop } from '../../../common/utils/misc';
-import { AttachRequestArguments, LaunchRequestArguments } from '../../types';
-import { ICustomDebugSessionEventHandlers } from './types';
-
-const eventName = 'ptvsd_subprocess';
-
-type ChildProcessLaunchData = {
-    rootProcessId: number;
-    initialProcessId: number;
-    rootStartRequest: {
-        // tslint:disable-next-line:no-banned-terms
-        arguments: LaunchRequestArguments | AttachRequestArguments;
-        command: 'attach' | 'request';
-        seq: number;
-        type: string;
-    };
-    parentProcessId: number;
-    processId: number;
-    port: number;
-};
+import { captureTelemetry } from '../../../telemetry';
+import { DEBUGGER_ATTACH_TO_CHILD_PROCESS } from '../../../telemetry/constants';
+import { AttachRequestArguments } from '../../types';
+import { ChildProcessLaunchData, IChildProcessAttachService } from './types';
 
+/**
+ * This class is responsible for attaching the debugger to any
+ * child processes launched. I.e. this is the classs responsible for multi-proc debugging.
+ * @export
+ * @class ChildProcessAttachEventHandler
+ * @implements {IChildProcessAttachService}
+ */
 @injectable()
-export class ChildProcessLaunchEventHandler implements ICustomDebugSessionEventHandlers {
+export class ChildProcessAttachService implements IChildProcessAttachService {
     constructor(@inject(IApplicationShell) private readonly appShell: IApplicationShell,
         @inject(IDebugService) private readonly debugService: IDebugService,
         @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService) { }
 
-    @swallowExceptions('Handle child process launch')
-    public async handleEvent(event: DebugSessionCustomEvent): Promise<void> {
-        if (!event || event.event !== eventName) {
-            return;
-        }
-        const data = event.body! as ChildProcessLaunchData;
+    @captureTelemetry(DEBUGGER_ATTACH_TO_CHILD_PROCESS)
+    public async attach(data: ChildProcessLaunchData): Promise<void> {
         const folder = this.getRelatedWorkspaceFolder(data);
         const debugConfig = this.getAttachConfiguration(data);
         const launched = await this.debugService.startDebugging(folder, debugConfig);
@@ -55,12 +42,11 @@ export class ChildProcessLaunchEventHandler implements ICustomDebugSessionEventH
         return this.workspaceService.workspaceFolders!.find(ws => ws.uri.fsPath === workspaceFolder);
     }
     protected getAttachConfiguration(data: ChildProcessLaunchData): AttachRequestArguments & DebugConfiguration {
+        const args = data.rootStartRequest.arguments;
         // tslint:disable-next-line:no-any
-        const config = JSON.parse(JSON.stringify(data.rootStartRequest.arguments)) as any as (AttachRequestArguments & DebugConfiguration);
+        const config = JSON.parse(JSON.stringify(args)) as any as (AttachRequestArguments & DebugConfiguration);
 
-        if (data.rootStartRequest.arguments.request === 'attach') {
-            config.host = data.rootStartRequest.arguments.host!;
-        }
+        config.host = args.request === 'attach' ? args.host! : 'localhost';
         config.port = data.port;
         config.name = `Child Process ${data.processId}`;
         config.request = 'attach';

src/client/debugger/extension/hooks/constants.ts

@@ -0,0 +1,12 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+export enum PTVSDEvents {
+    // Event sent by PTVSD when a child process is launched and ready to be attached to for multi-proc debugging.
+    ChildProcessLaunched = 'ptvsd_subprocess',
+
+    // Event sent by PTVSD when a process is started (identital to the `process` event in debugger protocol).
+    ProcessLaunched = 'ptvsd_process'
+}

src/client/debugger/extension/hooks/customEventHandlerDispatcher.ts → src/client/debugger/extension/hooks/eventHandlerDispatcher.ts

@@ -6,15 +6,18 @@
 import { inject, multiInject } from 'inversify';
 import { IDebugService } from '../../../common/application/types';
 import { IDisposableRegistry } from '../../../common/types';
-import { ICustomDebugSessionEventHandlers } from './types';
+import { IDebugSessionEventHandlers } from './types';
 
-export class CustomDebugSessionEventDispatcher {
-    constructor(@multiInject(ICustomDebugSessionEventHandlers) private readonly eventHandlers: ICustomDebugSessionEventHandlers[],
+export class DebugSessionEventDispatcher {
+    constructor(@multiInject(IDebugSessionEventHandlers) private readonly eventHandlers: IDebugSessionEventHandlers[],
         @inject(IDebugService) private readonly debugService: IDebugService,
         @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry) { }
     public registerEventHandlers() {
         this.disposables.push(this.debugService.onDidReceiveDebugSessionCustomEvent(e => {
-            this.eventHandlers.forEach(handler => handler.handleEvent(e).ignoreErrors());
+            this.eventHandlers.forEach(handler => handler.handleCustomEvent ? handler.handleCustomEvent(e).ignoreErrors() : undefined);
+        }));
+        this.disposables.push(this.debugService.onDidTerminateDebugSession(e => {
+            this.eventHandlers.forEach(handler => handler.handleTerminateEvent ? handler.handleTerminateEvent(e).ignoreErrors() : undefined);
         }));
     }
 }