From ac95032802f2d9f5a5ed88a9cc5a64232b488cba Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Wed, 29 Mar 2023 12:16:38 -0700 Subject: [PATCH] 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 https://github.com/microsoft/vscode/issues/173993 * fix multi target launch --- src/binder.ts | 564 +++++++++++++++-------- src/dap/connection.ts | 8 +- src/dapDebugServer.ts | 6 +- src/debugServer.ts | 4 +- src/flatSessionLauncher.ts | 5 +- src/targets/browser/browserAttacher.ts | 4 - src/targets/browser/browserLauncher.ts | 11 - src/targets/delegate/delegateLauncher.ts | 7 - src/targets/node/nodeLauncherBase.ts | 7 - src/targets/targets.ts | 5 - src/test/node/node-runtime.test.ts | 1 + src/test/test.ts | 19 +- src/vsDebugServer.ts | 7 +- 13 files changed, 382 insertions(+), 266 deletions(-) diff --git a/src/binder.ts b/src/binder.ts index bb7dfb42b..35d5d9d2a 100644 --- a/src/binder.ts +++ b/src/binder.ts @@ -12,11 +12,12 @@ import { DiagnosticToolSuggester } from './adapter/diagnosticToolSuggester'; import { SelfProfile } from './adapter/selfProfile'; import { Thread } from './adapter/threads'; import { CancellationTokenSource } from './common/cancellation'; +import { DisposableList } from './common/disposable'; import { EventEmitter, IDisposable } from './common/events'; import { ILogger, LogTag, resolveLoggerOptions } from './common/logging'; import { MutableLaunchConfig } from './common/mutableLaunchConfig'; -import { mapValues } from './common/objUtils'; -import { delay, getDeferred, IDeferred } from './common/promiseUtil'; +import { mapValues, once, truthy } from './common/objUtils'; +import { getDeferred } from './common/promiseUtil'; import * as urlUtils from './common/urlUtils'; import { AnyLaunchConfiguration, @@ -30,7 +31,7 @@ import DapConnection from './dap/connection'; import { createTargetContainer, provideLaunchParams } from './ioc'; import { disposeContainer, ExtensionLocation, IInitializeParams, IsVSCode } from './ioc-extras'; import { ITargetOrigin } from './targets/targetOrigin'; -import { ILauncher, ILaunchResult, ITarget } from './targets/targets'; +import { ILauncher, ILaunchResult, IStopMetadata, ITarget } from './targets/targets'; import { ITelemetryReporter } from './telemetry/telemetryReporter'; import { filterErrorsReportedToTelemetry, @@ -38,165 +39,182 @@ import { } from './telemetry/unhandledErrorReporter'; export interface IBinderDelegate { + /** + * Returns a promise that resolves to the DAP connection for the new target. + * Generally this involves calling back to the client to initialize the + * session, and then resolving once the client has provided a connection + * to return. + */ acquireDap(target: ITarget): Promise; - // Returns whether we should disable child session treatment. + + /** + * Handles launching of the session, returning `true` if the delegate + * handled it. If `false` is returned, the binder will wait for a launch/attach + * request on the target's DAP connection. + */ initAdapter(debugAdapter: DebugAdapter, target: ITarget, launcher: ILauncher): Promise; + + /** + * Called after a session is disconnected. + */ releaseDap(target: ITarget): void; } -type ThreadData = { thread: Thread; debugAdapter: DebugAdapter }; - +/** + * The Binder handles the lifecycle of a set of debug sessions. It's initialized + * with the root DAP, and can then handle new child sessions via calls of + * the `.boot()` method or launch/attach calls on the DAP connection. + * + * The provided delegate is responsible + * + * It manages a tree of sessions under the `_root`, which represents the top + * level "virtual" session. In some cases, more than one debug session can be + * created under the top level session, but more commonly there's a single + * session to debug a Node program or browser, for example. Under this, there + * may be other sessions. + * + * The binder makes an effort to ensure load and unload order is correct, such + * that parent sessions only send `terminated` or respond to the 'disconnect' + * request after all their child sessions have also entered the desired state. + * + * @see https://microsoft.github.io/debug-adapter-protocol/overview for control flows + */ export class Binder implements IDisposable { - private _delegate: IBinderDelegate; - private _disposables: IDisposable[]; - private _threads = new Map>(); - private _terminationCount = 0; - private _onTargetListChangedEmitter = new EventEmitter(); - readonly onTargetListChanged = this._onTargetListChangedEmitter.event; - private _dap: Promise; + private readonly _disposables = new DisposableList(); + private _dap: Dap.Api; private _targetOrigin: ITargetOrigin; private _launchParams?: AnyLaunchConfiguration; private _asyncStackPolicy?: IAsyncStackPolicy; private _serviceTree = new WeakMap(); - private _launchers?: ReadonlySet; + + /** Root of the session tree. Undefined until a launch/attach request is received. */ + private _root = new TreeNode(undefined); constructor( - delegate: IBinderDelegate, + private readonly _delegate: IBinderDelegate, connection: DapConnection, private readonly _rootServices: Container, targetOrigin: ITargetOrigin, ) { - this._delegate = delegate; this._dap = connection.dap(); this._targetOrigin = targetOrigin; - this._disposables = [ - this._onTargetListChangedEmitter, + this._disposables.callback(() => disposeContainer(_rootServices)); + this._disposables.push( installUnhandledErrorReporter( _rootServices.get(ILogger), _rootServices.get(ITelemetryReporter), _rootServices.get(IsVSCode), ), - ]; + ); connection.attachTelemetry(_rootServices.get(ITelemetryReporter)); - this._dap.then(dap => { - let lastBreakpointId = 0; - let selfProfile: SelfProfile | undefined; + const dap = this._dap; + let lastBreakpointId = 0; + let selfProfile: SelfProfile | undefined; - dap.on('initialize', async clientCapabilities => { - this._rootServices.bind(IInitializeParams).toConstantValue(clientCapabilities); - const capabilities = DebugAdapter.capabilities(); - if (clientCapabilities.clientID === 'vscode') { - filterErrorsReportedToTelemetry(); - } + dap.on('initialize', async clientCapabilities => { + this._rootServices.bind(IInitializeParams).toConstantValue(clientCapabilities); + const capabilities = DebugAdapter.capabilities(); + if (clientCapabilities.clientID === 'vscode') { + filterErrorsReportedToTelemetry(); + } - setTimeout(() => dap.initialized({}), 0); - return capabilities; - }); - dap.on('setExceptionBreakpoints', async () => ({})); - dap.on('setBreakpoints', async params => { - if (params.breakpoints?.length) { - _rootServices.get(DiagnosticToolSuggester).notifyHadBreakpoint(); - } + setTimeout(() => dap.initialized({}), 0); + return capabilities; + }); + dap.on('setExceptionBreakpoints', async () => ({})); + dap.on('setBreakpoints', async params => { + if (params.breakpoints?.length) { + _rootServices.get(DiagnosticToolSuggester).notifyHadBreakpoint(); + } - return { - breakpoints: - params.breakpoints?.map(() => ({ - id: ++lastBreakpointId, - verified: false, - message: l10n.t('Unbound breakpoint'), - })) ?? [], - }; // TODO: Put a useful message here - }); - dap.on('configurationDone', async () => ({})); - dap.on('threads', async () => ({ threads: [] })); - dap.on('loadedSources', async () => ({ sources: [] })); - dap.on('breakpointLocations', () => Promise.resolve({ breakpoints: [] })); - dap.on('attach', async params => { - await this.boot(params as AnyResolvingConfiguration, dap); - return {}; - }); - dap.on('launch', async params => { - await this.boot(params as AnyResolvingConfiguration, dap); - return {}; - }); - dap.on('pause', async () => { - return {}; - }); - dap.on('terminate', async () => { - await this._disconnect(); - return {}; - }); - dap.on('disconnect', async () => { - await this._disconnect(); - return {}; - }); - dap.on('restart', async ({ arguments: params }) => { - await this._restart(params as AnyResolvingConfiguration); - return {}; - }); - dap.on('startSelfProfile', async ({ file }) => { - selfProfile?.dispose(); - selfProfile = new SelfProfile(file); - await selfProfile.start(); - return {}; - }); - dap.on('stopSelfProfile', async () => { - if (selfProfile) { - await selfProfile.stop(); - selfProfile.dispose(); - selfProfile = undefined; - } + return { + breakpoints: + params.breakpoints?.map(() => ({ + id: ++lastBreakpointId, + verified: false, + message: l10n.t('Unbound breakpoint'), + })) ?? [], + }; + }); + dap.on('configurationDone', async () => ({})); + dap.on('threads', async () => ({ threads: [] })); + dap.on('loadedSources', async () => ({ sources: [] })); + dap.on('breakpointLocations', () => Promise.resolve({ breakpoints: [] })); + dap.on('attach', async params => { + await this.boot(params as AnyResolvingConfiguration, dap); + return {}; + }); + dap.on('launch', async params => { + await this.boot(params as AnyResolvingConfiguration, dap); + return {}; + }); + dap.on('pause', async () => { + return {}; + }); + dap.on('terminate', () => this._terminateRoot()); + dap.on('disconnect', () => this._disconnectRoot()); + dap.on('restart', async ({ arguments: params }) => { + await this._restart(params as AnyResolvingConfiguration); + return {}; + }); + dap.on('startSelfProfile', async ({ file }) => { + selfProfile?.dispose(); + selfProfile = new SelfProfile(file); + await selfProfile.start(); + return {}; + }); + dap.on('stopSelfProfile', async () => { + if (selfProfile) { + await selfProfile.stop(); + selfProfile.dispose(); + selfProfile = undefined; + } - return {}; - }); + return {}; }); } - private getLaunchers() { - if (!this._launchers) { - this._launchers = new Set(this._rootServices.getAll(ILauncher)); - - for (const launcher of this._launchers) { - launcher.onTargetListChanged( - () => { - const targets = this.targetList(); - this._attachToNewTargets(targets, launcher); - this._detachOrphanThreads(targets); - this._onTargetListChangedEmitter.fire(); - }, - undefined, - this._disposables, - ); - } + private readonly getLaunchers = once(() => { + const launchers = new Set(this._rootServices.getAll(ILauncher)); + + for (const launcher of launchers) { + this._disposables.push( + launcher.onTargetListChanged(() => { + const targets = this.targetList(); + this._attachToNewTargets(targets, launcher); + this._terminateOrphanThreads(targets); + }), + ); } - return this._launchers; + return launchers as ReadonlySet; + }); + + /** + * Terminates all running targets. Resolves when all have terminated. + */ + private async _terminateRoot() { + this._root.state = TargetState.Terminating; + await Promise.all([...this.getLaunchers()].map(l => l.terminate())); + await this._root.waitUntilChildrenAre(TargetState.Terminated); + this._root.state = TargetState.Terminated; + return {}; } - private async _disconnect() { - if (!this._launchers) { - return; + /** + * Disconnects all running targets. Resolves when all have disconnected. + */ + private async _disconnectRoot() { + if (this._root.state < TargetState.Terminating) { + await this._terminateRoot(); } this._rootServices.get(ITelemetryReporter).flush(); - await Promise.all([...this._launchers].map(l => l.disconnect())); - - const didTerminate = () => !this.targetList.length && this._terminationCount === 0; - if (didTerminate()) { - return; - } - - await new Promise(resolve => - this.onTargetListChanged(() => { - if (didTerminate()) { - resolve(); - } - }), - ); - - await delay(0); // next task so that we're sure terminated() sent + await this._root.waitUntilChildrenAre(TargetState.Disconnected); + this._root.state = TargetState.Disconnected; + return {}; } /** @@ -221,11 +239,14 @@ export class Binder implements IDisposable { if (params.rootPath) params.rootPath = urlUtils.platformPathToPreferredCase(params.rootPath); this._launchParams = params; - try { - await Promise.all([...this.getLaunchers()].map(l => this._launch(l, params, cts.token))); - } catch (e) { - throw e; - } + const boots = await Promise.all( + [...this.getLaunchers()].map(l => this._launch(l, params, cts.token)), + ); + + Promise.all(boots.map(b => b.terminated)).then(allMetadata => { + const metadata = allMetadata.find(truthy); + this._markTargetAsTerminated(this._root, { restart: !!metadata?.restart }); + }); return {}; } @@ -287,25 +308,22 @@ export class Binder implements IDisposable { launcher: ILauncher, params: AnyLaunchConfiguration, cancellationToken: CancellationToken, - ): Promise { + ) { const result = await this.captureLaunch(launcher, params, cancellationToken); if (!result.blockSessionTermination) { - return; + return { terminated: Promise.resolve(undefined) }; } - ++this._terminationCount; - - const listener = launcher.onTerminated(result => { - listener.dispose(); - const detach = this._detachOrphanThreads(this.targetList(), { restart: result.restart }); - --this._terminationCount; - this._onTargetListChangedEmitter.fire(); - if (!this._terminationCount) { - detach.then(() => this._dap).then(dap => dap.terminated({ restart: result.restart })); - } - }); - - this._disposables.push(listener); + return { + terminated: new Promise(resolve => { + const listener = this._disposables.push( + launcher.onTerminated(result => { + listener.dispose(); + resolve(result); + }), + ); + }), + }; } /** @@ -326,7 +344,7 @@ export class Binder implements IDisposable { telemetryReporter: this._rootServices.get(ITelemetryReporter), cancellationToken, targetOrigin: this._targetOrigin, - dap: await this._dap, + dap: this._dap, }); } catch (e) { this._rootServices.get(ILogger).warn(LogTag.RuntimeLaunch, 'Launch returned error', { @@ -348,10 +366,8 @@ export class Binder implements IDisposable { } dispose() { - for (const disposable of this._disposables) disposable.dispose(); - this._disposables = []; - disposeContainer(this._rootServices); - this._detachOrphanThreads([]); + this._disposables.dispose(); + this._terminateOrphanThreads([]); } targetList(): ITarget[] { @@ -363,11 +379,12 @@ export class Binder implements IDisposable { return result; } - public async attach(target: ITarget, threadData: IDeferred, launcher: ILauncher) { + private async attach(node: TargetTreeNode, launcher: ILauncher) { if (!this._launchParams) { throw new Error('Cannot launch before params have been set'); } + const target = node.value; if (!target.canAttach()) { return; } @@ -376,7 +393,7 @@ export class Binder implements IDisposable { return; } const connection = await this._delegate.acquireDap(target); - const dap = await connection.dap(); + const dap = connection.dap(); const launchParams = this._launchParams; if (!this._asyncStackPolicy) { @@ -390,25 +407,28 @@ export class Binder implements IDisposable { connection.attachTelemetry(container.get(ITelemetryReporter)); this._serviceTree.set(target, parentContainer); - // todo: move scriptskipper into services collection const debugAdapter = new DebugAdapter(dap, this._asyncStackPolicy, launchParams, container); const thread = debugAdapter.createThread(cdp, target); const startThread = async () => { await debugAdapter.launchBlocker(); target.runIfWaitingForDebugger(); - threadData.resolve({ thread, debugAdapter }); + node.threadData.resolve({ thread, debugAdapter }); return {}; }; // default disconnect/terminate/restart handlers that can be overridden // by the delegate in initAdapter() - dap.on('disconnect', () => this.detachTarget(target, container)); - dap.on('terminate', () => this.stopTarget(target, container)); + dap.on('disconnect', args => this._disconnectTarget(node, args)); + dap.on('terminate', () => this._terminateTarget(node)); dap.on('restart', async () => { - if (target.canRestart()) target.restart(); - else await this._restart(); - return {}; + if (target.canRestart()) { + target.restart(); + } else { + await this._restart(); + } + + return Promise.resolve({}); }); if (await this._delegate.initAdapter(debugAdapter, target, launcher)) { @@ -421,79 +441,117 @@ export class Binder implements IDisposable { await target.afterBind(); } + private _attachToNewTargets(targets: ITarget[], launcher: ILauncher) { + for (const target of targets.values()) { + if (!target.waitingForDebugger()) { + continue; + } + + if (TreeNode.targetNodes.has(target)) { + continue; + } + + const parentTarget = target.parent(); + const parent = parentTarget ? TreeNode.targetNodes.get(parentTarget) : this._root; + if (!parent) { + throw new Error(`Got target with unknown parent: ${target.name()}`); + } + + const node = new TreeNode(target) as TargetTreeNode; + parent.add(node); + this.attach(node, launcher); + } + } + /** - * Called when we get a disconnect for a target. We stop the - * specific target if we can, otherwise we just tear down the session. + * Terminates all targets in the tree that aren't in the `targets` list. */ - private async detachTarget(target: ITarget, container: Container) { - container.get(ITelemetryReporter).flush(); - if (!this.targetList().includes(target)) { + private async _terminateOrphanThreads( + targets: ITarget[], + terminateArgs?: Dap.TerminatedEventParams, + ) { + const toRelease = [...this._root.all()].filter(n => !targets.includes(n.value)); + return Promise.all(toRelease.map(n => this._markTargetAsTerminated(n, terminateArgs))); + } + + /** + * Marks the target as terminated, called when the target lists change. + */ + private async _markTargetAsTerminated( + node: TreeNode, + terminateArgs: Dap.TerminatedEventParams = {}, + ) { + if (node.state >= TargetState.Terminating) { + await node.waitUntil(TargetState.Terminated); return {}; } - if (target.canDetach()) { - await target.detach(); - this._releaseTarget(target); + node.state = TargetState.Terminating; + if (isTargetTreeNode(node)) { + const threadData = await node.threadData.promise; + await threadData.thread.dispose(); + threadData.debugAdapter.dap.terminated(terminateArgs); + threadData.debugAdapter.dispose(); } else { - this._disconnect(); + this._dap.terminated(terminateArgs); } + await node.waitUntilChildrenAre(TargetState.Terminated); + node.state = TargetState.Terminated; return {}; } /** - * Called when we get a terminate for a target. We stop the - * specific target if we can, otherwise we just tear down the session. + * DAP method call to terminate a target. Resolves once the target and + * all of its children have terminated. + * + * This doesn't actually mark a node as terminated; that's done when the + * target is removed from the targets list as `_markTargetAsTerminated` is called. */ - private stopTarget(target: ITarget, container: Container) { - container.get(ITelemetryReporter).flush(); - if (!this.targetList().includes(target)) { - return Promise.resolve({}); + private async _terminateTarget(node: TargetTreeNode) { + if (node.state >= TargetState.Disconnected) { + return {}; } - if (target.canStop()) { - target.stop(); + if (node.value.canStop()) { + node.value.stop(); } else { - this._disconnect(); + this._terminateRoot(); } - return Promise.resolve({}); + await node.waitUntil(TargetState.Terminated); + return {}; } - private _attachToNewTargets(targets: ITarget[], launcher: ILauncher) { - for (const target of targets.values()) { - if (!target.waitingForDebugger()) { - continue; - } + /** + * DAP method call to disconnect a target. Resolves once the target and + * all of its children have disconnected. + */ + private async _disconnectTarget(node: TargetTreeNode, args: Dap.DisconnectParams) { + if (node.state >= TargetState.Disconnected) { + return {}; + } - if (!this._threads.has(target)) { - const threadData = getDeferred(); - this._threads.set(target, threadData); - this.attach(target, threadData, launcher); + if (node.state < TargetState.Terminating) { + if (args.terminateDebuggee === false && node.value.canDetach()) { + await node.value.detach(); + } else if (node.value.canStop()) { + node.value.stop(); + } else { + this._terminateRoot(); } } - } - private async _detachOrphanThreads( - targets: ITarget[], - terminateArgs?: Dap.TerminatedEventParams, - ) { - await Promise.all( - [...this._threads.keys()] - .filter(target => !targets.includes(target)) - .map(target => this._releaseTarget(target, terminateArgs)), - ); - } + await node.waitUntil(TargetState.Terminated); + await node.waitUntilChildrenAre(TargetState.Disconnected); + node.state = TargetState.Disconnected; + queueMicrotask(() => this._delegate.releaseDap(node.value)); + + const parentTarget = node.value.parent(); + const parentNode = parentTarget ? TreeNode.targetNodes.get(parentTarget) : this._root; + parentNode?.remove(node); - async _releaseTarget(target: ITarget, terminateArgs: Dap.TerminatedEventParams = {}) { - const data = this._threads.get(target); - if (!data) return; - this._threads.delete(target); - const threadData = await data.promise; - await threadData.thread.dispose(); - threadData.debugAdapter.dap.terminated(terminateArgs); - threadData.debugAdapter.dispose(); - this._delegate.releaseDap(target); + return {}; } } @@ -505,3 +563,101 @@ function warnNightly(dap: Dap.Api): void { }); } } + +const enum TargetState { + Running, + Terminating, + Terminated, + Disconnected, +} + +type ThreadData = { thread: Thread; debugAdapter: DebugAdapter }; + +type TargetTreeNode = TreeNode & { value: ITarget }; + +export const isTargetTreeNode = (node: TreeNode): node is TargetTreeNode => !!node.value; + +/** + * Node in the tree that manages the collective state of shutdowns, so that + * terminations and disconnections happen gracefully in-order. + */ +class TreeNode { + public static targetNodes = new WeakMap(); + public readonly threadData = getDeferred(); + + private _state = TargetState.Running; + private readonly _children = new Set(); + private readonly _stateChangeEmitter = new EventEmitter(); + + public get state() { + return this._state; + } + + public set state(state: TargetState) { + if (state > this._state) { + this._state = state; + this._stateChangeEmitter.fire(state); + } + } + + /** Value is only undefined for the root node */ + constructor(public readonly value: ITarget | undefined) { + if (value) { + TreeNode.targetNodes.set(value, this); + } + } + + /** + * Adds a new child to the target tree. + */ + public add(child: TreeNode) { + this._children.add(child); + } + + /** + * Removes a child to the target tree. + */ + public remove(child: TreeNode) { + this._children.delete(child); + } + + /** + * Returns a promise that resolves when this node has reached at least the + * given state in the lifecycle. + */ + public async waitUntil(state: TargetState) { + if (this._state >= state) { + return Promise.resolve(); + } + + return new Promise(resolve => { + const l = this._stateChangeEmitter.event(s => { + if (s >= state) { + l.dispose(); + resolve(); + } + }); + }); + } + + /** + * Returns a promise that resolves when all children this node have reached + * at least the given state. + */ + public async waitUntilChildrenAre(state: TargetState) { + await Promise.all([...this._children].map(c => c.waitUntil(state))); + } + + /** + * Returns an iterator that lists all targets in the tree. + */ + public *all(): IterableIterator { + if (isTargetTreeNode(this)) { + yield this; + } + + for (const child of this._children) { + yield* child.all(); + } + } +} diff --git a/src/dap/connection.ts b/src/dap/connection.ts index 89dd2022d..0d0b3ebc4 100644 --- a/src/dap/connection.ts +++ b/src/dap/connection.ts @@ -33,7 +33,7 @@ export default class Connection { private _pendingRequests = new Map void>(); private _requestHandlers = new Map Promise>(); private _eventListeners = new Map void>>(); - private _dap: Promise; + private _dap: Dap.Api; private disposables: IDisposable[] = []; private _initialized = getDeferred(); @@ -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 { + public dap(): Dap.Api { return this._dap; } diff --git a/src/dapDebugServer.ts b/src/dapDebugServer.ts index 7f7c9a1a6..26e942700 100644 --- a/src/dapDebugServer.ts +++ b/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) { diff --git a/src/debugServer.ts b/src/debugServer.ts index f09c4e012..238cc9500 100644 --- a/src/debugServer.ts +++ b/src/debugServer.ts @@ -25,9 +25,9 @@ class Configurator { private _customBreakpoints = new Set(); private lastBreakpointId = 0; - constructor(dapPromise: Promise) { + constructor(dap: Dap.Api) { this._setBreakpointsParams = []; - dapPromise.then(dap => this._listen(dap)); + this._listen(dap); } _listen(dap: Dap.Api) { diff --git a/src/flatSessionLauncher.ts b/src/flatSessionLauncher.ts index 99bbcfc94..b9e963976 100644 --- a/src/flatSessionLauncher.ts +++ b/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; diff --git a/src/targets/browser/browserAttacher.ts b/src/targets/browser/browserAttacher.ts index e4c66bcf6..dac6bc194 100644 --- a/src/targets/browser/browserAttacher.ts +++ b/src/targets/browser/browserAttacher.ts @@ -274,10 +274,6 @@ export class BrowserAttacher< this._connection?.close(); } - public disconnect(): Promise { - return this.terminate(); - } - async restart(): Promise { if (!this._targetManager) { return; diff --git a/src/targets/browser/browserLauncher.ts b/src/targets/browser/browserLauncher.ts index de9298c27..32e94b35e 100644 --- a/src/targets/browser/browserLauncher.ts +++ b/src/targets/browser/browserLauncher.ts @@ -265,17 +265,6 @@ export abstract class BrowserLauncher * @inheritdoc */ public async terminate(): Promise { - for (const target of this.targetList() as BrowserTarget[]) { - if (target.type() === BrowserTargetType.Page) { - await target.cdp().Page.close({}); - } - } - } - - /** - * @inheritdoc - */ - public async disconnect(): Promise { await this._targetManager?.closeBrowser(); } diff --git a/src/targets/delegate/delegateLauncher.ts b/src/targets/delegate/delegateLauncher.ts index 59ab14c0f..74f593c25 100644 --- a/src/targets/delegate/delegateLauncher.ts +++ b/src/targets/delegate/delegateLauncher.ts @@ -131,13 +131,6 @@ export class DelegateLauncher implements ILauncher { return Promise.resolve(); } - /** - * @inheritdoc - */ - public disconnect(): Promise { - return this.terminate(); - } - /** * @inheritdoc */ diff --git a/src/targets/node/nodeLauncherBase.ts b/src/targets/node/nodeLauncherBase.ts index 0e78c9477..6282abc3b 100644 --- a/src/targets/node/nodeLauncherBase.ts +++ b/src/targets/node/nodeLauncherBase.ts @@ -177,13 +177,6 @@ export abstract class NodeLauncherBase implement } } - /** - * @inheritdoc - */ - public async disconnect(): Promise { - await this.terminate(); - } - /** * Restarts the ongoing program. */ diff --git a/src/targets/targets.ts b/src/targets/targets.ts index cb8e06b23..363409ff8 100644 --- a/src/targets/targets.ts +++ b/src/targets/targets.ts @@ -139,11 +139,6 @@ export interface ILauncher extends IDisposable { */ terminate(): Promise; - /** - * Disconnects from the debugged process. This should be idempotent. - */ - disconnect(): Promise; - /** * Attempts to restart the debugged process. This may no-op for certain * debug types, like attach. diff --git a/src/test/node/node-runtime.test.ts b/src/test/node/node-runtime.test.ts index 38e66d86e..60b78feec 100644 --- a/src/test/node/node-runtime.test.ts +++ b/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); diff --git a/src/test/test.ts b/src/test/test.ts index c29d40c85..fd13f37c1 100644 --- a/src/test/test.ts +++ b/src/test/test.ts @@ -103,7 +103,6 @@ class Session { } async _init(): Promise { - 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(f => (this._launchCallback = f)); + await result.load(); return result as NodeTestHandle; } diff --git a/src/vsDebugServer.ts b/src/vsDebugServer.ts index 511356453..709ffdee6 100644 --- a/src/vsDebugServer.ts +++ b/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;