feat: adopt supportTerminateDebuggee for browsers and node (#1777)

Fixes #1733 on the way

Author: connor4312

CHANGELOG.md

@@ -5,6 +5,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 ## Nightly (only)
 
 - feat: add `Symbol.for("debug.description")` as a way to generate object descriptions ([vscode#102181](https://github.com/microsoft/vscode/issues/102181))
+- feat: adopt supportTerminateDebuggee for browsers and node ([#1733](https://github.com/microsoft/vscode-js-debug/issues/1733))
 - fix: child processes from extension host not getting spawned during debug
 - fix: support vite HMR source replacements ([#1761](https://github.com/microsoft/vscode-js-debug/issues/1761))
 - fix: immediately log stdout/err unless EXT is encountered ([vscode#181785](https://github.com/microsoft/vscode/issues/181785))

src/adapter/debugAdapter.ts

@@ -247,7 +247,7 @@ export class DebugAdapter implements IDisposable {
       supportsExceptionOptions: false,
       supportsValueFormattingOptions: true,
       supportsExceptionInfoRequest: true,
-      supportTerminateDebuggee: false,
+      supportTerminateDebuggee: true,
       supportsDelayedStackTraceLoading: true,
       supportsLoadedSourcesRequest: true,
       supportsLogPoints: true,

src/binder.ts

@@ -153,8 +153,8 @@ export class Binder implements IDisposable {
     dap.on('pause', async () => {
       return {};
     });
-    dap.on('terminate', () => this._terminateRoot());
-    dap.on('disconnect', () => this._disconnectRoot());
+    dap.on('terminate', () => this._terminateRoot(true));
+    dap.on('disconnect', args => this._disconnectRoot(args));
     dap.on('restart', async ({ arguments: params }) => {
       await this._restart(params as AnyResolvingConfiguration);
       return {};
@@ -195,9 +195,9 @@ export class Binder implements IDisposable {
   /**
    * Terminates all running targets. Resolves when all have terminated.
    */
-  private async _terminateRoot() {
+  private async _terminateRoot(terminateDebuggee?: boolean) {
     this._root.state = TargetState.Terminating;
-    await Promise.all([...this.getLaunchers()].map(l => l.terminate()));
+    await Promise.all([...this.getLaunchers()].map(l => l.terminate(terminateDebuggee)));
     await this._root.waitUntilChildrenAre(TargetState.Terminated);
     this._root.state = TargetState.Terminated;
     return {};
@@ -206,9 +206,9 @@ export class Binder implements IDisposable {
   /**
    * Disconnects all running targets. Resolves when all have disconnected.
    */
-  private async _disconnectRoot() {
+  private async _disconnectRoot(args: Dap.DisconnectParams) {
     if (this._root.state < TargetState.Terminating) {
-      await this._terminateRoot();
+      await this._terminateRoot(args.terminateDebuggee);
     }
 
     this._rootServices.get<ITelemetryReporter>(ITelemetryReporter).flush();
@@ -516,7 +516,7 @@ export class Binder implements IDisposable {
     if (node.value.canStop()) {
       node.value.stop();
     } else {
-      this._terminateRoot();
+      this._terminateRoot(true);
     }
 
     await node.waitUntil(TargetState.Terminated);
@@ -533,12 +533,12 @@ export class Binder implements IDisposable {
     }
 
     if (node.state < TargetState.Terminating) {
-      if (args.terminateDebuggee === false && node.value.canDetach()) {
+      if (args.terminateDebuggee !== true && node.value.canDetach()) {
         await node.value.detach();
       } else if (node.value.canStop()) {
         node.value.stop();
       } else {
-        this._terminateRoot();
+        this._terminateRoot(args.terminateDebuggee);
       }
     }
 

src/targets/node/nodeAttacher.ts

@@ -15,11 +15,12 @@ import { AnyLaunchConfiguration, INodeAttachConfiguration } from '../../configur
 import { retryGetNodeEndpoint } from '../browser/spawn/endpoints';
 import { ISourcePathResolverFactory } from '../sourcePathResolverFactory';
 import { IStopMetadata } from '../targets';
+import { killTree } from './killTree';
 import { LeaseFile } from './lease-file';
 import { NodeAttacherBase } from './nodeAttacherBase';
 import { watchAllChildren } from './nodeAttacherCluster';
 import { INodeBinaryProvider, NodeBinary } from './nodeBinaryProvider';
-import { IRunData } from './nodeLauncherBase';
+import { IProcessTelemetry, IRunData } from './nodeLauncherBase';
 import { IProgram, StubProgram, WatchDogProgram } from './program';
 import { IRestartPolicy, RestartPolicyFactory } from './restartPolicy';
 import { WatchDog } from './watchdogSpawn';
@@ -33,6 +34,8 @@ import { WatchDog } from './watchdogSpawn';
  */
 @injectable()
 export class NodeAttacher extends NodeAttacherBase<INodeAttachConfiguration> {
+  private telemetry?: IProcessTelemetry;
+
   constructor(
     @inject(INodeBinaryProvider) pathProvider: INodeBinaryProvider,
     @inject(ILogger) logger: ILogger,
@@ -136,10 +139,18 @@ export class NodeAttacher extends NodeAttacherBase<INodeAttachConfiguration> {
     return doLaunch(this.restarters.create(runData.params.restart));
   }
 
+  public override async terminate(terminateDebuggee?: boolean): Promise<void> {
+    await super.terminate();
+
+    if (terminateDebuggee && this.telemetry) {
+      killTree(this.telemetry.processId, this.logger);
+    }
+  }
+
   /**
    * @override
    */
-  protected createLifecycle(
+  protected override createLifecycle(
     cdp: Cdp.Api,
     run: IRunData<INodeAttachConfiguration>,
     target: Cdp.Target.TargetInfo,
@@ -177,12 +188,21 @@ export class NodeAttacher extends NodeAttacherBase<INodeAttachConfiguration> {
     const leaseFile = new LeaseFile();
     await leaseFile.startTouchLoop();
 
-    const binary = await this.resolveNodePath(run.params);
+    let binary = new NodeBinary('node', undefined);
+    try {
+      binary = await this.resolveNodePath(run.params);
+    } catch {
+      // ignored -- not really a big deal, just used to find capabilities for
+      // attach params, but all Node versions post-12 will have the same behavior.
+    }
+
     const [telemetry] = await Promise.all([
       this.gatherTelemetryFromCdp(cdp, run),
       this.setEnvironmentVariables(cdp, run, leaseFile.path, parentInfo.targetId, binary),
     ]);
 
+    this.telemetry = telemetry;
+
     if (telemetry && run.params.attachExistingChildren) {
       watchAllChildren(
         {

src/targets/node/nodeLauncherBase.ts

@@ -518,7 +518,7 @@ export abstract class NodeLauncherBase<T extends AnyNodeConfiguration> implement
   protected async gatherTelemetryFromCdp(
     cdp: Cdp.Api,
     run: IRunData<T>,
-  ): Promise<IProcessTelemetry | void> {
+  ): Promise<IProcessTelemetry | undefined> {
     for (let retries = 0; retries < 8; retries++) {
       const telemetry = await cdp.Runtime.evaluate({
         contextId: 1,

src/targets/targets.ts

@@ -136,8 +136,11 @@ export interface ILauncher extends IDisposable {
 
   /**
    * Terminates the debugged process. This should be idempotent.
+   * @param terminateDebuggee Whether the debugee should be terminated as well.
+   * For `launch` type requests, this has no effect, but attach requests should
+   * use "stop" logic instead of "disconnect" logic when provided.
    */
-  terminate(): Promise<void>;
+  terminate(terminateDebuggee?: boolean): Promise<void>;
 
   /**
    * Attempts to restart the debugged process. This may no-op for certain

src/test/infra/infra-initialize.txt

@@ -25,7 +25,7 @@
             supportsCondition : true
         }
     ]
-    supportTerminateDebuggee : false
+    supportTerminateDebuggee : true
     supportedChecksumAlgorithms : [
     ]
     supportsBreakpointLocationsRequest : true