refactor: improve and order handling in the Binder (#1619)

* refactor: improve and order handling in the `Binder`

Previously the `Binder` was a little messy. The binder managed the
"tree" of debug sessions, but teardown was not handled in a particular
order. So, for example, when terminating a Node.js session with a bunch
of child processes, the parent process could be terminated and
disconnected while its children were nominally still "running" if their
async shutdown logic didn't finish yet.

Now that things are nicely ordered, I can fix microsoft/vscode#173993

* fix multi target launch

src/dap/connection.ts

@@ -33,7 +33,7 @@ export default class Connection {
   private _pendingRequests = new Map<number, (result: string | object) => void>();
   private _requestHandlers = new Map<string, (params: object) => Promise<object>>();
   private _eventListeners = new Map<string, Set<(params: object) => void>>();
-  private _dap: Promise<Dap.Api>;
+  private _dap: Dap.Api;
   private disposables: IDisposable[] = [];
 
   private _initialized = getDeferred<Connection>();
@@ -52,15 +52,15 @@ export default class Connection {
     this.disposables.push(
       this.transport.messageReceived(event => this._onMessage(event.message, event.receivedTime)),
     );
-    this._dap = Promise.resolve(this._createApi());
+    this._dap = this._createApi();
   }
 
   public attachTelemetry(telemetryReporter: ITelemetryReporter) {
     this.telemetryReporter = telemetryReporter;
-    this._dap.then(dap => telemetryReporter.attachDap(dap));
+    telemetryReporter.attachDap(this._dap);
   }
 
-  public dap(): Promise<Dap.Api> {
+  public dap(): Dap.Api {
     return this._dap;
   }
 

src/dapDebugServer.ts

@@ -13,7 +13,7 @@ import { DebugAdapter } from './adapter/debugAdapter';
 import { Binder, IBinderDelegate } from './binder';
 import { ILogger } from './common/logging';
 import { ProxyLogger } from './common/logging/proxyLogger';
-import { getDeferred, IDeferred } from './common/promiseUtil';
+import { IDeferred, getDeferred } from './common/promiseUtil';
 import { AnyResolvingConfiguration } from './configuration';
 import Dap from './dap/api';
 import DapConnection from './dap/connection';
@@ -154,7 +154,7 @@ class DapSessionManager implements IBinderDelegate {
       if (!parentCnx) {
         throw new Error('Expected parent session to have a settled value');
       }
-      dap = await parentCnx.connection.dap();
+      dap = parentCnx.connection.dap();
     } else {
       dap = this.dapRoot;
     }
@@ -234,7 +234,7 @@ function startDebugServer(options: net.ListenOptions) {
         const logger = new ProxyLogger();
         const transport = new StreamDapTransport(socket, socket, logger);
         const connection = new DapConnection(transport, logger);
-        const dap = await connection.dap();
+        const dap = connection.dap();
 
         const initialized = await collectInitialize(dap);
         if ('__pendingTargetId' in initialized.launchParams) {

src/debugServer.ts

@@ -25,9 +25,9 @@ class Configurator {
   private _customBreakpoints = new Set<string>();
   private lastBreakpointId = 0;
 
-  constructor(dapPromise: Promise<Dap.Api>) {
+  constructor(dap: Dap.Api) {
     this._setBreakpointsParams = [];
-    dapPromise.then(dap => this._listen(dap));
+    this._listen(dap);
   }
 
   _listen(dap: Dap.Api) {

src/flatSessionLauncher.ts

@@ -45,10 +45,7 @@ class VSDebugSession implements IDebugSessionLike {
     this._name = newName;
     this.childConnection
       .then(conn => conn.initializedBlocker)
-      .then(conn => conn.dap())
-      .then(dap => {
-        dap.process({ name: newName });
-      });
+      .then(conn => conn.dap().process({ name: newName }));
   }
   get name() {
     return this._name;

src/targets/browser/browserAttacher.ts

@@ -274,10 +274,6 @@ export class BrowserAttacher<
     this._connection?.close();
   }
 
-  public disconnect(): Promise<void> {
-    return this.terminate();
-  }
-
   async restart(): Promise<void> {
     if (!this._targetManager) {
       return;

src/targets/browser/browserLauncher.ts

@@ -265,17 +265,6 @@ export abstract class BrowserLauncher<T extends AnyChromiumLaunchConfiguration>
    * @inheritdoc
    */
   public async terminate(): Promise<void> {
-    for (const target of this.targetList() as BrowserTarget[]) {
-      if (target.type() === BrowserTargetType.Page) {
-        await target.cdp().Page.close({});
-      }
-    }
-  }
-
-  /**
-   * @inheritdoc
-   */
-  public async disconnect(): Promise<void> {
     await this._targetManager?.closeBrowser();
   }
 

src/targets/delegate/delegateLauncher.ts

@@ -131,13 +131,6 @@ export class DelegateLauncher implements ILauncher {
     return Promise.resolve();
   }
 
-  /**
-   * @inheritdoc
-   */
-  public disconnect(): Promise<void> {
-    return this.terminate();
-  }
-
   /**
    * @inheritdoc
    */

src/targets/node/nodeLauncherBase.ts

@@ -177,13 +177,6 @@ export abstract class NodeLauncherBase<T extends AnyNodeConfiguration> implement
     }
   }
 
-  /**
-   * @inheritdoc
-   */
-  public async disconnect(): Promise<void> {
-    await this.terminate();
-  }
-
   /**
    * Restarts the ongoing program.
    */

src/targets/targets.ts

@@ -139,11 +139,6 @@ export interface ILauncher extends IDisposable {
    */
   terminate(): Promise<void>;
 
-  /**
-   * Disconnects from the debugged process. This should be idempotent.
-   */
-  disconnect(): Promise<void>;
-
   /**
    * Attempts to restart the debugged process. This may no-op for certain
    * debug types, like attach.

src/test/node/node-runtime.test.ts

@@ -446,6 +446,7 @@ describe('node runtime', () => {
 
       const handle = await r.attachNode(0, { port, restart: true });
       await handle.dap.once('stopped');
+      await handle.dap.disconnect({});
       await r.rootDap().disconnect({});
 
       await delay(1000);

src/test/test.ts

@@ -103,7 +103,6 @@ class Session {
   }
 
   async _init(): Promise<Dap.InitializeResult> {
-    await this.adapterConnection.dap();
     const [r] = await Promise.all([
       this.dap.initialize({
         clientID: 'pwa-test',
@@ -372,14 +371,13 @@ export class TestRoot {
 
     this.logger = services.get(ILogger);
     this._root = new Session(this.logger);
-    this._root.adapterConnection.dap().then(dap => {
-      dap.on('initialize', async () => {
-        dap.initialized({});
-        return DebugAdapter.capabilities();
-      });
-      dap.on('configurationDone', async () => {
-        return {};
-      });
+    const dap = this._root.adapterConnection.dap();
+    dap.on('initialize', async () => {
+      dap.initialized({});
+      return DebugAdapter.capabilities();
+    });
+    dap.on('configurationDone', async () => {
+      return {};
     });
 
     this.binder = new Binder(this, this._root.adapterConnection, services, new TargetOrigin('0'));
@@ -558,7 +556,8 @@ export class TestRoot {
       __workspaceFolder: this._workspaceRoot,
       ...options,
     } as INodeAttachConfiguration);
-    const result = await new Promise(f => (this._launchCallback = f));
+    const result = await new Promise<ITestHandle>(f => (this._launchCallback = f));
+    await result.load();
     return result as NodeTestHandle;
   }
 

src/vsDebugServer.ts

@@ -15,7 +15,7 @@ import 'reflect-metadata';
 import { Readable, Writable } from 'stream';
 import { DebugConfiguration } from 'vscode';
 import { DebugType } from './common/contributionUtils';
-import { getDeferred, IDeferred } from './common/promiseUtil';
+import { IDeferred, getDeferred } from './common/promiseUtil';
 import { IPseudoAttachConfiguration } from './configuration';
 import DapConnection from './dap/connection';
 import { createGlobalContainer } from './ioc';
@@ -44,10 +44,7 @@ class VSDebugSession implements IDebugSessionLike {
     this._name = newName;
     this.childConnection
       .then(conn => conn.initializedBlocker)
-      .then(conn => conn.dap())
-      .then(dap => {
-        dap.process({ name: newName });
-      });
+      .then(conn => conn.dap().process({ name: newName }));
   }
   get name() {
     return this._name;