chore: add pid tracker for browser (#2264)

Author: connor4312

src/build/dapCustom.ts

@@ -340,6 +340,12 @@ const dapCustom: JSONSchema4 = {
           },
         },
       },
+    }, {
+      properties: {
+        pid: {
+          type: 'number',
+        },
+      },
     }),
 
     ...makeRequest(

src/common/pidLiveness.ts

@@ -0,0 +1,46 @@
+/*---------------------------------------------------------
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ *--------------------------------------------------------*/
+
+import { IDisposable } from './disposable';
+
+/**
+ * Utility to poll a PID for liveness and notify when it exits.
+ */
+export class PidLiveness implements IDisposable {
+  private timer?: NodeJS.Timeout;
+
+  constructor(
+    public readonly pid: number,
+    private readonly onExit: (killed: boolean) => void,
+    pollInterval: number,
+  ) {
+    this.updateInterval(pollInterval);
+  }
+
+  public dispose() {
+    clearInterval(this.timer);
+  }
+
+  public updateInterval(interval = 200) {
+    if (this.timer) {
+      clearInterval(this.timer);
+    }
+
+    this.timer = setInterval(() => {
+      if (!isProcessAlive(this.pid)) {
+        this.onExit(true);
+        this.dispose();
+      }
+    }, interval);
+  }
+}
+
+function isProcessAlive(processId: number) {
+  try {
+    process.kill(processId, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}

src/dap/api.d.ts

@@ -90,7 +90,7 @@ export namespace Dap {
     output(params: OutputEventParams): void;
 
     /**
-     * The event indicates that some information about a breakpoint has changed.
+     * The event indicates that some information about a breakpoint has changed. While debug adapters may notify the clients of `changed` breakpoints using this event, clients should continue to use the breakpoint's original properties when updating a source's breakpoints in the `breakpoint` request.
      */
     breakpoint(params: BreakpointEventParams): void;
 
@@ -1300,7 +1300,7 @@ export namespace Dap {
     ): Promise<OutputEventParams>;
 
     /**
-     * The event indicates that some information about a breakpoint has changed.
+     * The event indicates that some information about a breakpoint has changed. While debug adapters may notify the clients of `changed` breakpoints using this event, clients should continue to use the breakpoint's original properties when updating a source's breakpoints in the `breakpoint` request.
      */
     on(request: 'breakpoint', handler: (params: BreakpointEventParams) => void): void;
     off(request: 'breakpoint', handler: (params: BreakpointEventParams) => void): void;
@@ -2919,6 +2919,7 @@ export namespace Dap {
   }
 
   export interface LaunchUnelevatedResult {
+    pid?: number;
   }
 
   export interface LaunchVSCodeParams {

src/targets/browser/launcher.ts

@@ -25,6 +25,7 @@ import {
   ChildProcessBrowserProcess,
   IBrowserProcess,
   NonTrackedBrowserProcess,
+  PidTrackedBrowserProcess,
 } from './spawn/browserProcess';
 import { retryGetBrowserEndpoint } from './spawn/endpoints';
 import { launchUnelevatedChrome } from './unelevatedChome';
@@ -104,13 +105,15 @@ export async function launch(
     clientCapabilities.supportsLaunchUnelevatedProcessRequest && options.launchUnelevated
   );
   if (launchUnelevated && typeof actualConnection === 'number' && actualConnection !== 0) {
-    await launchUnelevatedChrome(
+    const response = await launchUnelevatedChrome(
       dap,
       executablePath,
       browserArguments.toArray(),
       cancellationToken,
     );
-    browserProcess = new NonTrackedBrowserProcess(logger);
+    browserProcess = response.pid
+      ? new PidTrackedBrowserProcess(response.pid, logger)
+      : new NonTrackedBrowserProcess(logger);
   } else {
     logger.info(LogTag.RuntimeLaunch, `Launching Chrome from ${executablePath}`);
 

src/targets/browser/spawn/browserProcess.ts

@@ -13,6 +13,7 @@ import { TaskCancelledError } from '../../../common/cancellation';
 import { DisposableList } from '../../../common/disposable';
 import { EventEmitter } from '../../../common/events';
 import { ILogger } from '../../../common/logging';
+import { PidLiveness } from '../../../common/pidLiveness';
 import { delay } from '../../../common/promiseUtil';
 import { KillBehavior } from '../../../configuration';
 import { killTree } from '../../node/killTree';
@@ -116,6 +117,42 @@ export class NonTrackedBrowserProcess implements IBrowserProcess {
   }
 }
 
+/**
+ * Browser process tracked using PID liveness polling (used for unelevated launches).
+ */
+export class PidTrackedBrowserProcess implements IBrowserProcess {
+  public readonly stderr = undefined;
+  public readonly stdout = undefined;
+
+  private readonly exitEmitter = new EventEmitter<number>();
+  public readonly onExit = this.exitEmitter.event;
+
+  private readonly errorEmitter = new EventEmitter<Error>();
+  public readonly onError = this.errorEmitter.event;
+
+  private readonly liveness: PidLiveness;
+
+  constructor(private readonly pid: number, private readonly logger: ILogger) {
+    this.liveness = new PidLiveness(pid, () => this.exitEmitter.fire(0), 1000);
+  }
+
+  public async transport(
+    options: ITransportOptions,
+    cancellationToken: CancellationToken,
+  ): Promise<ITransport> {
+    return inspectWsConnection(this.logger, this, options, cancellationToken);
+  }
+
+  public kill(): void {
+    try {
+      process.kill(this.pid);
+    } catch {
+      // ignore
+    }
+    this.liveness.updateInterval(200);
+  }
+}
+
 /**
  * Browser process
  */

src/targets/browser/unelevatedChome.ts

@@ -10,11 +10,11 @@ export async function launchUnelevatedChrome(
   chromePath: string,
   chromeArgs: string[],
   cancellationToken: CancellationToken,
-): Promise<void> {
+): Promise<{ pid?: number }> {
   const response = dap.launchUnelevatedRequest({
     process: chromePath,
     args: chromeArgs,
   });
 
-  await timeoutPromise(response, cancellationToken, 'Could not launch browser unelevated');
+  return await timeoutPromise(response, cancellationToken, 'Could not launch browser unelevated');
 }

src/targets/node/program.ts

@@ -4,6 +4,7 @@
 
 import { ChildProcess } from 'child_process';
 import { ILogger } from '../../common/logging';
+import { PidLiveness } from '../../common/pidLiveness';
 import { KillBehavior } from '../../configuration';
 import Dap from '../../dap/api';
 import { IStopMetadata } from '../targets';
@@ -140,15 +141,19 @@ export class TerminalProcess implements IProgram {
       resolve({ code: 0, killed });
     }),
   );
-  private loop?: { timer: NodeJS.Timeout; processId: number };
+  private liveness?: PidLiveness;
 
   constructor(
     private readonly terminalResult: Dap.RunInTerminalResult,
     private readonly logger: ILogger,
     private readonly killBehavior: KillBehavior,
   ) {
     if (terminalResult.processId) {
-      this.startPollLoop(terminalResult.processId);
+      this.liveness = new PidLiveness(
+        terminalResult.processId,
+        this.onStopped,
+        TerminalProcess.terminationPollInterval,
+      );
     }
   }
 
@@ -158,9 +163,7 @@ export class TerminalProcess implements IProgram {
       return; // to avoid any races
     }
 
-    if (!this.loop) {
-      this.startPollLoop(processId);
-    }
+    this.liveness ??= new PidLiveness(processId, this.onStopped, 1000);
   }
 
   public stop(): Promise<IStopMetadata> {
@@ -171,46 +174,22 @@ export class TerminalProcess implements IProgram {
 
     // If we're already polling some process ID, kill it and accelerate polling
     // so we can confirm it's dead quickly.
-    if (this.loop) {
-      killTree(this.loop.processId, this.logger, this.killBehavior);
-      this.startPollLoop(this.loop.processId, TerminalProcess.killConfirmInterval);
+    if (this.liveness) {
+      killTree(this.liveness.pid, this.logger, this.killBehavior);
+      this.liveness.updateInterval(TerminalProcess.killConfirmInterval);
     } else if (this.terminalResult.shellProcessId) {
       // If we had a shell process ID, well, that's good enough.
       killTree(this.terminalResult.shellProcessId, this.logger, this.killBehavior);
-      this.startPollLoop(this.terminalResult.shellProcessId, TerminalProcess.killConfirmInterval);
+      this.liveness = new PidLiveness(
+        this.terminalResult.shellProcessId,
+        this.onStopped,
+        TerminalProcess.killConfirmInterval,
+      );
     } else {
       // Otherwise, we can't do anything. Pretend like we did.
       this.onStopped(true);
     }
 
     return this.stopped;
   }
-
-  private startPollLoop(processId: number, interval = TerminalProcess.terminationPollInterval) {
-    if (this.loop) {
-      clearInterval(this.loop.timer);
-    }
-
-    const loop = {
-      processId,
-      timer: setInterval(() => {
-        if (!isProcessAlive(processId)) {
-          clearInterval(loop.timer);
-          this.onStopped(true);
-        }
-      }, interval),
-    };
-
-    this.loop = loop;
-  }
-}
-
-function isProcessAlive(processId: number) {
-  try {
-    // kill with signal=0 just test for whether the proc is alive. It throws if not.
-    process.kill(processId, 0);
-    return true;
-  } catch {
-    return false;
-  }
 }