fix: pausing on first line of worker_thread when created with empty env

Fixes microsoft/vscode#125451
microsoft · Jul 7, 2021 · ec8bf1e · ec8bf1e
1 parent 3148806
commit ec8bf1e
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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<void>;
-  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<number, ExecutionContext> = 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<void> {
-    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
@@ -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
@@ -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<Cdp.Api | undefined> = Promise.resolve(undefined);

diff --git 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<void>().event;
   private attached = false;
   private isWaitingForDebugger = true;