fix: race in entrypoint breakpoints (#1360)

* fix: race in entrypoint breakpoints This was causing a ~10% failure in a test, and could have caused some failures for users in edge cases (where rarely stepping into the first function or class declaration in a file would not work.) * chore: update vscode-test for resiliency, use newer macos image * chore: use node 16 in CI to match vscode
2022-08-04 16:12:10 -07:00 · 2022-08-04 16:12:10 -07:00 · cbb1086812
--- a/.ci/pipeline.yml
+++ b/.ci/pipeline.yml
@ -6,11 +6,11 @@ jobs:
 - job: macOS
  timeoutInMinutes: 20
  pool:
-    vmImage: 'macOS-10.15'
+    vmImage: 'macos-latest'
  steps:
  - template: common-validation.yml
  variables:
-    node_version: 14.18.3
+    node_version: 16.14.2

 - job: Linux
  pool:
@ -18,7 +18,7 @@ jobs:
  steps:
  - template: common-validation.yml
  variables:
-    node_version: 14.18.3
+    node_version: 16.14.2

 - job: LinuxMinspec
  pool:
@ -26,7 +26,7 @@ jobs:
  steps:
  - template: common-validation.yml
  variables:
-    node_version: 14.18.3
+    node_version: 16.14.2
    only_minspec: true

 - job: Windows
@ -35,4 +35,4 @@ jobs:
  steps:
  - template: common-validation.yml
  variables:
-    node_version: 14.18.3
+    node_version: 16.14.2
--- a/package-lock.json
+++ b/package-lock.json
@ -76,7 +76,7 @@
        "@types/ws": "^8.5.3",
        "@typescript-eslint/eslint-plugin": "^5.17.0",
        "@typescript-eslint/parser": "^5.17.0",
-        "@vscode/test-electron": "^2.1.3",
+        "@vscode/test-electron": "^2.1.5",
        "chai": "^4.3.6",
        "chai-as-promised": "^7.1.1",
        "chai-string": "^1.5.0",
@ -2317,9 +2317,9 @@
      }
    },
    "node_modules/@vscode/test-electron": {
-      "version": "2.1.3",
-      "resolved": "https://registry.npmjs.org/@vscode/test-electron/-/test-electron-2.1.3.tgz",
-      "integrity": "sha512-ps/yJ/9ToUZtR1dHfWi1mDXtep1VoyyrmGKC3UnIbScToRQvbUjyy1VMqnMEW3EpMmC3g7+pyThIPtPyCLHyow==",
+      "version": "2.1.5",
+      "resolved": "https://registry.npmjs.org/@vscode/test-electron/-/test-electron-2.1.5.tgz",
+      "integrity": "sha512-O/ioqFpV+RvKbRykX2ItYPnbcZ4Hk5V0rY4uhQjQTLhGL9WZUvS7exzuYQCCI+ilSqJpctvxq2llTfGXf9UnnA==",
      "dev": true,
      "dependencies": {
        "http-proxy-agent": "^4.0.1",
@ -18927,9 +18927,9 @@
      }
    },
    "@vscode/test-electron": {
-      "version": "2.1.3",
-      "resolved": "https://registry.npmjs.org/@vscode/test-electron/-/test-electron-2.1.3.tgz",
-      "integrity": "sha512-ps/yJ/9ToUZtR1dHfWi1mDXtep1VoyyrmGKC3UnIbScToRQvbUjyy1VMqnMEW3EpMmC3g7+pyThIPtPyCLHyow==",
+      "version": "2.1.5",
+      "resolved": "https://registry.npmjs.org/@vscode/test-electron/-/test-electron-2.1.5.tgz",
+      "integrity": "sha512-O/ioqFpV+RvKbRykX2ItYPnbcZ4Hk5V0rY4uhQjQTLhGL9WZUvS7exzuYQCCI+ilSqJpctvxq2llTfGXf9UnnA==",
      "dev": true,
      "requires": {
        "http-proxy-agent": "^4.0.1",
@ -19141,15 +19141,13 @@
      "version": "1.8.0",
      "resolved": "https://registry.npmjs.org/acorn-import-assertions/-/acorn-import-assertions-1.8.0.tgz",
      "integrity": "sha512-m7VZ3jwz4eK6A4Vtt8Ew1/mNbP24u0FhdyfA7fSvnJR6LMdfOYnmuIrrJAgrYfYJ10F/otaHTtrtrtmHdMNzEw==",
-      "dev": true,
-      "requires": {}
+      "dev": true
    },
    "acorn-jsx": {
      "version": "5.3.2",
      "resolved": "https://registry.npmjs.org/acorn-jsx/-/acorn-jsx-5.3.2.tgz",
      "integrity": "sha512-rq9s+JNhf0IChjtDXxllJ7g41oZk5SlXtp0LHwyA5cejwn7vKmKp4pPri6YEePv2PU65sAsegbXtIinmDFDXgQ==",
-      "dev": true,
-      "requires": {}
+      "dev": true
    },
    "acorn-loose": {
      "version": "8.3.0",
@ -19200,8 +19198,7 @@
      "version": "3.5.2",
      "resolved": "https://registry.npmjs.org/ajv-keywords/-/ajv-keywords-3.5.2.tgz",
      "integrity": "sha512-5p6WTN0DdTGVQk6VjcEju19IgaHudalcfabD7yhDGeA6bcQnmL+CpveLJq/3hvfwd1aof6L386Ougkx6RfyMIQ==",
-      "dev": true,
-      "requires": {}
+      "dev": true
    },
    "ansi-colors": {
      "version": "1.1.0",
@ -20171,8 +20168,7 @@
      "version": "1.5.0",
      "resolved": "https://registry.npmjs.org/chai-string/-/chai-string-1.5.0.tgz",
      "integrity": "sha512-sydDC3S3pNAQMYwJrs6dQX0oBQ6KfIPuOZ78n7rocW0eJJlsHPh2t3kwW7xfwYA/1Bf6/arGtSUo16rxR2JFlw==",
-      "dev": true,
-      "requires": {}
+      "dev": true
    },
    "chai-subset": {
      "version": "1.6.0",
@ -21597,8 +21593,7 @@
      "version": "3.1.1",
      "resolved": "https://registry.npmjs.org/eslint-plugin-header/-/eslint-plugin-header-3.1.1.tgz",
      "integrity": "sha512-9vlKxuJ4qf793CmeeSrZUvVClw6amtpghq3CuWcB5cUNnWHQhgcqy5eF8oVKFk1G3Y/CbchGfEaw3wiIJaNmVg==",
-      "dev": true,
-      "requires": {}
+      "dev": true
    },
    "eslint-plugin-react": {
      "version": "7.29.4",
@ -23803,8 +23798,7 @@
      "version": "5.1.0",
      "resolved": "https://registry.npmjs.org/icss-utils/-/icss-utils-5.1.0.tgz",
      "integrity": "sha512-soFhflCVWLfRNOPU3iv5Z9VUdT44xFRbzjLsEzSr5AQmgqPMTHdU3PMT1Cf1ssx8fLNJDA1juftYl+PUcv3MqA==",
-      "dev": true,
-      "requires": {}
+      "dev": true
    },
    "ieee754": {
      "version": "1.2.1",
@ -27468,8 +27462,7 @@
          "version": "8.4.2",
          "resolved": "https://registry.npmjs.org/ws/-/ws-8.4.2.tgz",
          "integrity": "sha512-Kbk4Nxyq7/ZWqr/tarI9yIt/+iNNFOjBXEWgTb4ydaNHBNGgvf2QHbS9fdfsndfjFlFwEd4Al+mw83YkaD10ZA==",
-          "dev": true,
-          "requires": {}
+          "dev": true
        }
      }
    },
@ -27512,8 +27505,7 @@
      "version": "3.0.0",
      "resolved": "https://registry.npmjs.org/postcss-modules-extract-imports/-/postcss-modules-extract-imports-3.0.0.tgz",
      "integrity": "sha512-bdHleFnP3kZ4NYDhuGlVK+CMrQ/pqUm8bx/oGL93K6gVwiclvX5x0n76fYMKuIGKzlABOy13zsvqjb0f92TEXw==",
-      "dev": true,
-      "requires": {}
+      "dev": true
    },
    "postcss-modules-local-by-default": {
      "version": "4.0.0",
@ -29009,8 +29001,7 @@
      "version": "3.3.1",
      "resolved": "https://registry.npmjs.org/style-loader/-/style-loader-3.3.1.tgz",
      "integrity": "sha512-GPcQ+LDJbrcxHORTRes6Jy2sfvK2kS6hpSfI/fXhPt+spVzxF6LJ1dHLN9zIGmVaaP044YKaIatFaufENRiDoQ==",
-      "dev": true,
-      "requires": {}
+      "dev": true
    },
    "supports-color": {
      "version": "5.4.0",
@ -30194,8 +30185,7 @@
          "version": "7.5.7",
          "resolved": "https://registry.npmjs.org/ws/-/ws-7.5.7.tgz",
          "integrity": "sha512-KMvVuFzpKBuiIXW3E4u3mySRO2/mCHSyZDJQM5NQ9Q9KHWHWh0NHgfbRMLLrceUK5qAL4ytALJbpRMjixFZh8A==",
-          "dev": true,
-          "requires": {}
+          "dev": true
        }
      }
    },
@ -30310,8 +30300,7 @@
    "ws": {
      "version": "8.5.0",
      "resolved": "https://registry.npmjs.org/ws/-/ws-8.5.0.tgz",
-      "integrity": "sha512-BWX0SWVgLPzYwF8lTzEy1egjhS4S4OEAHfsO8o65WOVsrnSRGaSiUaa9e0ggGlkMTtBlmOpEXiie9RUcBO86qg==",
-      "requires": {}
+      "integrity": "sha512-BWX0SWVgLPzYwF8lTzEy1egjhS4S4OEAHfsO8o65WOVsrnSRGaSiUaa9e0ggGlkMTtBlmOpEXiie9RUcBO86qg=="
    },
    "xdg-default-browser": {
      "version": "2.1.0",
--- a/package.json
+++ b/package.json
@ -122,7 +122,7 @@
    "@types/ws": "^8.5.3",
    "@typescript-eslint/eslint-plugin": "^5.17.0",
    "@typescript-eslint/parser": "^5.17.0",
-    "@vscode/test-electron": "^2.1.3",
+    "@vscode/test-electron": "^2.1.5",
    "chai": "^4.3.6",
    "chai-as-promised": "^7.1.1",
    "chai-string": "^1.5.0",
--- a/src/adapter/breakpoints/breakpointBase.ts
+++ b/src/adapter/breakpoints/breakpointBase.ts
@ -217,6 +217,13 @@ export abstract class Breakpoint {
    cdpId: Cdp.Debugger.BreakpointId,
    resolvedLocations: readonly Cdp.Debugger.Location[],
  ) {
+    // Update with the resolved locations immediately and synchronously. This
+    // prevents a race conditions where a source is parsed immediately before
+    // a breakpoint it hit and not returning correctly in `BreakpointManager.isEntrypointBreak`.
+    // This _can_ be fairly prevelant, especially when resolving UI locations
+    // involves loading or waiting for source maps.
+    this.updateExistingCdpRef(cdpId, bp => ({ ...bp, locations: resolvedLocations }));
+
    const uiLocation = (
      await Promise.all(
        resolvedLocations.map(l => thread.rawLocationToUiLocation(thread.rawLocation(l))),
@ -232,20 +239,15 @@ export abstract class Breakpoint {
      return;
    }

-    this.updateCdpRefs(list =>
-      list.map(bp => {
-        if (bp.state !== CdpReferenceState.Applied || bp.cdpId !== cdpId) {
-          return bp;
-        }
-        const locations = this._manager._sourceContainer.currentSiblingUiLocations(uiLocation);
-        const inPreferredSource = locations.filter(l => l.source === source);
-        return {
-          ...bp,
-          locations: resolvedLocations,
-          uiLocations: inPreferredSource.length ? inPreferredSource : locations,
-        };
-      }),
-    );
+    this.updateExistingCdpRef(cdpId, bp => {
+      const locations = this._manager._sourceContainer.currentSiblingUiLocations(uiLocation);
+      const inPreferredSource = locations.filter(l => l.source === source);
+      return {
+        ...bp,
+        locations: resolvedLocations,
+        uiLocations: inPreferredSource.length ? inPreferredSource : locations,
+      };
+    });
  }

  /**
@ -377,6 +379,20 @@ export abstract class Breakpoint {
    return undefined;
  }

+  /**
+   * Updates an existing applied CDP breakpoint, by its CDP ID.
+   */
+  protected updateExistingCdpRef(
+    cdpId: string,
+    mutator: (l: Readonly<BreakpointCdpReference>) => Readonly<BreakpointCdpReference>,
+  ) {
+    this.updateCdpRefs(list =>
+      list.map(bp =>
+        bp.state !== CdpReferenceState.Applied || bp.cdpId !== cdpId ? bp : mutator(bp),
+      ),
+    );
+  }
+
  /**
   * Updates the list of CDP breakpoint references. Used to provide lifecycle
   * hooks to consumers and internal caches.
--- a/src/test/node/node-runtime-sets-arguments.txt
+++ b/src/test/node/node-runtime-sets-arguments.txt
@ -8,6 +8,6 @@
    0: '--some'
    1: 'very fancy'
    2: '--arguments'
-    > __proto__: Array(0)
-    > __proto__: Object
    length: 3
+    > [[Prototype]]: Array(0)
+    > [[Prototype]]: Object
--- a/src/test/node/node-runtime.test.ts
+++ b/src/test/node/node-runtime.test.ts
@ -497,10 +497,6 @@ describe('node runtime', () => {
  });

  itIntegrates('sets arguments', async ({ r }) => {
-    if (process.platform === 'darwin') {
-      return; // the ADO runner on Darwin seems to use the wrong Node version
-    }
-
    createFileTree(testFixturesDir, { 'test.js': 'debugger' });
    const handle = await r.runScript('test.js', {
      args: ['--some', 'very fancy', '--arguments'],