From ec8bf1e1949559b218099dc2e876ab303f2ba6f7 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Wed, 7 Jul 2021 15:28:38 -0700 Subject: [PATCH] fix: pausing on first line of worker_thread when created with empty env Fixes https://github.com/microsoft/vscode/issues/125451 --- CHANGELOG.md | 3 +- src/adapter/debugAdapter.ts | 7 ++-- src/adapter/threads.ts | 59 ++++++++++++--------------- src/targets/browser/browserTargets.ts | 3 +- src/targets/node/nodeTarget.ts | 3 +- src/targets/node/nodeWorkerTarget.ts | 3 +- 6 files changed, 36 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d038da9e2..31d8c36e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he ## Nightly -Nothing (yet) +- fix: pausing on first line of worker_thread when created with empty env ([ref](https://github.com/microsoft/vscode/issues/125451)) +- chore: adopt new terminal icon ## v1.58 (June 2021) diff --git a/src/adapter/debugAdapter.ts b/src/adapter/debugAdapter.ts index a9f6a867d..0f3bd2b75 100644 --- a/src/adapter/debugAdapter.ts +++ b/src/adapter/debugAdapter.ts @@ -15,6 +15,7 @@ import { AnyLaunchConfiguration } from '../configuration'; import Dap from '../dap/api'; import * as errors from '../dap/errors'; import { disposeContainer } from '../ioc-extras'; +import { ITarget } from '../targets/targets'; import { ITelemetryReporter } from '../telemetry/telemetryReporter'; import { IAsyncStackPolicy } from './asyncStackPolicy'; import { BreakpointManager } from './breakpoints'; @@ -31,7 +32,7 @@ import { BasicCpuProfiler } from './profiling/basicCpuProfiler'; import { ScriptSkipper } from './scriptSkipper/implementation'; import { IScriptSkipper } from './scriptSkipper/scriptSkipper'; import { ISourceWithMap, SourceContainer, SourceFromMap } from './sources'; -import { IThreadDelegate, Thread } from './threads'; +import { Thread } from './threads'; import { VariableStore } from './variables'; const localize = nls.loadMessageBundle(); @@ -304,12 +305,12 @@ export class DebugAdapter implements IDisposable { return errors.createSilentError(localize('error.threadNotFound', 'Thread not found')); } - createThread(cdp: Cdp.Api, delegate: IThreadDelegate): Thread { + createThread(cdp: Cdp.Api, target: ITarget): Thread { this._thread = new Thread( this.sourceContainer, cdp, this.dap, - delegate, + target, this._services.get(IRenameProvider), this._services.get(ILogger), this._services.get(IEvaluator), diff --git a/src/adapter/threads.ts b/src/adapter/threads.ts index edf90a9cf..b56004332 100644 --- a/src/adapter/threads.ts +++ b/src/adapter/threads.ts @@ -18,7 +18,8 @@ import { AnyLaunchConfiguration, IChromiumBaseConfiguration, OutputSource } from import Dap from '../dap/api'; import * as errors from '../dap/errors'; import * as ProtocolError from '../dap/protocolError'; -import { IBreakpointPathAndId } from '../targets/targets'; +import { NodeWorkerTarget } from '../targets/node/nodeWorkerTarget'; +import { ITarget } from '../targets/targets'; import { BreakpointManager, EntryBreakpointMode } from './breakpoints'; import { UserDefinedBreakpoint } from './breakpoints/userDefinedBreakpoint'; import { ICompletions } from './completions'; @@ -99,15 +100,6 @@ export type Script = { resolvedSource?: Source; }; -export interface IThreadDelegate { - name(): string; - supportsCustomBreakpoints(): boolean; - scriptUrlToUrl(url: string): string; - executionContextName(description: Cdp.Runtime.ExecutionContextDescription): string; - initialize(): Promise; - entryBreakpoint?: IBreakpointPathAndId; -} - export type ScriptWithSourceMapHandler = ( script: Script, sources: Source[], @@ -148,7 +140,6 @@ export class Thread implements IVariableStoreDelegate { private _pausedVariables?: VariableStore; private _pausedForSourceMapScriptId?: string; private _executionContexts: Map = new Map(); - private _delegate: IThreadDelegate; readonly replVariables: VariableStore; private _sourceContainer: SourceContainer; private _pauseOnSourceMapBreakpointId?: Cdp.Debugger.BreakpointId; @@ -180,7 +171,7 @@ export class Thread implements IVariableStoreDelegate { sourceContainer: SourceContainer, cdp: Cdp.Api, dap: Dap.Api, - delegate: IThreadDelegate, + private readonly target: ITarget, renameProvider: IRenameProvider, private readonly logger: ILogger, private readonly evaluator: IEvaluator, @@ -191,7 +182,6 @@ export class Thread implements IVariableStoreDelegate { private readonly exceptionPause: IExceptionPauseService, ) { this._dap = new DeferredContainer(dap); - this._delegate = delegate; this._sourceContainer = sourceContainer; this._cdp = cdp; this.id = Thread._lastThreadId++; @@ -212,7 +202,7 @@ export class Thread implements IVariableStoreDelegate { } name(): string { - return this._delegate.name(); + return this.target.name(); } pausedDetails(): IPausedDetails | undefined { @@ -390,7 +380,7 @@ export class Thread implements IVariableStoreDelegate { const prefix = params.text.slice(0, params.column).trim(); return [...this._executionContexts.values()] - .map(c => `cd ${this._delegate.executionContextName(c.description)}`) + .map(c => `cd ${this.target.executionContextName(c.description)}`) .filter(label => label.startsWith(prefix)) .map(label => ({ label, start: 0, length: params.text.length })); } @@ -410,7 +400,7 @@ export class Thread implements IVariableStoreDelegate { if (args.context === 'repl' && args.expression.startsWith('cd ')) { const contextName = args.expression.substring('cd '.length).trim(); for (const ec of this._executionContexts.values()) { - if (this._delegate.executionContextName(ec.description) === contextName) { + if (this.target.executionContextName(ec.description) === contextName) { this._selectedContext = ec; return { result: `[${contextName}]`, @@ -507,7 +497,7 @@ export class Thread implements IVariableStoreDelegate { } else { const contextName = this._selectedContext && this.defaultExecutionContext() !== this._selectedContext - ? `\x1b[33m[${this._delegate.executionContextName(this._selectedContext.description)}] ` + ? `\x1b[33m[${this.target.executionContextName(this._selectedContext.description)}] ` : ''; const resultVar = await this.replVariables.createVariable(response.result, 'repl'); return { @@ -566,7 +556,7 @@ export class Thread implements IVariableStoreDelegate { this.logger.info(LogTag.RuntimeLaunch, 'Running with noDebug, so debug domains are disabled'); } - this._delegate.initialize(); + this.target.initialize(); this._dap.with(dap => dap.thread({ @@ -627,6 +617,8 @@ export class Thread implements IVariableStoreDelegate { const hitBreakpoints = (event.hitBreakpoints ?? []).filter( bp => bp !== this._pauseOnSourceMapBreakpointId, ); + // "Break on start" is not actually a by-spec reason in CDP, it's added on from Node.js, so cast `as string`: + // https://github.com/nodejs/node/blob/9cbf6af5b5ace0cc53c1a1da3234aeca02522ec6/src/node_contextify.cc#L913 const isInspectBrk = (event.reason as string) === 'Break on start'; const location = event.callFrames[0].location; const scriptId = event.data?.scriptId || location.scriptId; @@ -659,7 +651,7 @@ export class Thread implements IVariableStoreDelegate { await this._breakpointManager.shouldPauseAt( event, hitBreakpoints, - this._delegate.entryBreakpoint, + this.target.entryBreakpoint, true, ) ) { @@ -675,7 +667,7 @@ export class Thread implements IVariableStoreDelegate { : await this._breakpointManager.shouldPauseAt( event, hitBreakpoints, - this._delegate.entryBreakpoint, + this.target.entryBreakpoint, false, ); @@ -684,15 +676,18 @@ export class Thread implements IVariableStoreDelegate { } } - // "Break on start" is not actually a by-spec reason in CDP, it's added on from Node.js, so cast `as string`: - // https://github.com/nodejs/node/blob/9cbf6af5b5ace0cc53c1a1da3234aeca02522ec6/src/node_contextify.cc#L913 - if ( - isInspectBrk && - (('continueOnAttach' in this.launchConfig && this.launchConfig.continueOnAttach) || - this.launchConfig.type === DebugType.ExtensionHost) - ) { - this.resume(); - return; + if (isInspectBrk) { + if ( + // Continue if continueOnAttach is requested... + ('continueOnAttach' in this.launchConfig && this.launchConfig.continueOnAttach) || + // Or if we're debugging an extension host... + this.launchConfig.type === DebugType.ExtensionHost || + // Or if the target is a worker_thread https://github.com/microsoft/vscode/issues/125451 + this.target instanceof NodeWorkerTarget + ) { + this.resume(); + return; + } } // We store pausedDetails in a local variable to avoid race conditions while awaiting this._smartStepper.shouldSmartStep @@ -873,7 +868,7 @@ export class Thread implements IVariableStoreDelegate { } async updateCustomBreakpoint(id: CustomBreakpointId, enabled: boolean): Promise { - if (!this._delegate.supportsCustomBreakpoints()) return; + if (!this.target.supportsCustomBreakpoints()) return; const breakpoint = customBreakpoints().get(id); if (!breakpoint) return; // Do not fail for custom breakpoints, to account for @@ -997,7 +992,7 @@ export class Thread implements IVariableStoreDelegate { default: if (event.hitBreakpoints && event.hitBreakpoints.length) { let isStopOnEntry = false; // By default we assume breakpoints aren't stop on entry - const userEntryBp = this._delegate.entryBreakpoint; + const userEntryBp = this.target.entryBreakpoint; if (userEntryBp && event.hitBreakpoints.includes(userEntryBp.cdpId)) { isStopOnEntry = true; // But if it matches the entry breakpoint id, then it's probably stop on entry const entryBreakpointSource = this._sourceContainer.source({ @@ -1098,7 +1093,7 @@ export class Thread implements IVariableStoreDelegate { return; } - if (event.url) event.url = this._delegate.scriptUrlToUrl(event.url); + if (event.url) event.url = this.target.scriptUrlToUrl(event.url); let urlHashMap = this._scriptSources.get(event.url); if (!urlHashMap) { diff --git a/src/targets/browser/browserTargets.ts b/src/targets/browser/browserTargets.ts index 5b37fcca9..05082dd41 100644 --- a/src/targets/browser/browserTargets.ts +++ b/src/targets/browser/browserTargets.ts @@ -3,7 +3,6 @@ *--------------------------------------------------------*/ import { URL } from 'url'; -import { IThreadDelegate } from '../../adapter/threads'; import Cdp from '../../cdp/api'; import { EventEmitter } from '../../common/events'; import { ILogger } from '../../common/logging'; @@ -55,7 +54,7 @@ const stoppableTypes = restartableTypes; export type PauseOnExceptionsState = 'none' | 'uncaught' | 'all'; -export class BrowserTarget implements ITarget, IThreadDelegate { +export class BrowserTarget implements ITarget { readonly parentTarget: BrowserTarget | undefined; private _manager: BrowserTargetManager; private _cdp: Cdp.Api; diff --git a/src/targets/node/nodeTarget.ts b/src/targets/node/nodeTarget.ts index 4eb1a9574..94b42e751 100644 --- a/src/targets/node/nodeTarget.ts +++ b/src/targets/node/nodeTarget.ts @@ -3,7 +3,6 @@ *--------------------------------------------------------*/ import { basename } from 'path'; -import { IThreadDelegate } from '../../adapter/threads'; import Cdp from '../../cdp/api'; import Connection from '../../cdp/connection'; import { EventEmitter } from '../../common/events'; @@ -28,7 +27,7 @@ export interface INodeTargetLifecycleHooks { close?(target: NodeTarget): void; } -export class NodeTarget implements ITarget, IThreadDelegate { +export class NodeTarget implements ITarget { private _cdp: Cdp.Api; private _targetName: string; private _serialize: Promise = Promise.resolve(undefined); diff --git a/src/targets/node/nodeWorkerTarget.ts b/src/targets/node/nodeWorkerTarget.ts index 7a7b193e1..b481683d4 100644 --- a/src/targets/node/nodeWorkerTarget.ts +++ b/src/targets/node/nodeWorkerTarget.ts @@ -2,7 +2,6 @@ * Copyright (C) Microsoft Corporation. All rights reserved. *--------------------------------------------------------*/ -import { IThreadDelegate } from '../../adapter/threads'; import Cdp from '../../cdp/api'; import { EventEmitter } from '../../common/events'; import { ILogger } from '../../common/logging'; @@ -13,7 +12,7 @@ import { ITargetOrigin } from '../targetOrigin'; import { ITarget } from '../targets'; import { NodeTarget } from './nodeTarget'; -export class NodeWorkerTarget implements ITarget, IThreadDelegate { +export class NodeWorkerTarget implements ITarget { public readonly onNameChanged = new EventEmitter().event; private attached = false; private isWaitingForDebugger = true;