debugger: wait for target startup before initialization

The debugger endpoint can accept a connection before the target has
entered its startup wait. In that window,Runtime.runIfWaitingForDebugger
may be handled too early, leaving the target waiting indefinitely.

Use NodeRuntime.waitingForDebugger as a readiness handshake before
initializing the debugger domains and releasing the target. Apply the
handshake to both the interactive debugger and probe mode,and reject the
wait if the session closes.

Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>

Author: Archkon

lib/internal/debugger/inspect_helpers.js

@@ -4,7 +4,9 @@ const {
   ArrayPrototypePushApply,
   Number,
   Promise,
+  PromiseWithResolvers,
   RegExpPrototypeExec,
+  SafePromiseRace,
   StringPrototypeEndsWith,
 } = primordials;
 
@@ -18,7 +20,10 @@ const {
   AbortController,
 } = require('internal/abort_controller');
 
-const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes;
+const {
+  ERR_DEBUGGER_ERROR,
+  ERR_DEBUGGER_STARTUP_ERROR,
+} = require('internal/errors').codes;
 const {
   exitCodes: {
     kInvalidCommandLineArgument,
@@ -61,6 +66,41 @@ function ensureTrailingNewline(text) {
   return StringPrototypeEndsWith(text, '\n') ? text : `${text}\n`;
 }
 
+async function waitForDebugger(
+  client,
+  callMethod = (method) => client.callMethod(method),
+) {
+  const {
+    promise: waitingPromise,
+    resolve: resolveWaiting,
+  } = PromiseWithResolvers();
+  const {
+    promise: closedPromise,
+    reject: rejectClosed,
+  } = PromiseWithResolvers();
+  const onWaiting = () => resolveWaiting();
+  const onClose = () => {
+    rejectClosed(new ERR_DEBUGGER_ERROR(
+      'Debugger session ended while waiting for target startup'));
+  };
+
+  client.once('NodeRuntime.waitingForDebugger', onWaiting);
+  client.once('close', onClose);
+  try {
+    await SafePromiseRace([
+      callMethod('NodeRuntime.enable'),
+      closedPromise,
+    ]);
+    await SafePromiseRace([
+      waitingPromise,
+      closedPromise,
+    ]);
+  } finally {
+    client.removeListener('NodeRuntime.waitingForDebugger', onWaiting);
+    client.removeListener('close', onClose);
+  }
+}
+
 function writeInspectUsageAndExit(invokedAs, message, exitCode) {
   const code = exitCode ?? (message ? kInvalidCommandLineArgument : 0);
   const out = code === 0 ? process.stdout : process.stderr;
@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
 module.exports = {
   ensureTrailingNewline,
   launchChildProcess,
+  waitForDebugger,
   writeInspectUsageAndExit,
 };

lib/internal/debugger/inspect_probe.js

@@ -33,6 +33,7 @@ const InspectClient = require('internal/debugger/inspect_client');
 const {
   ensureTrailingNewline,
   launchChildProcess,
+  waitForDebugger,
 } = require('internal/debugger/inspect_helpers');
 
 const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes;
@@ -1044,11 +1045,16 @@ class ProbeInspectorSession {
       this.connected = true;
 
       try {
+        await waitForDebugger(
+          this.client,
+          (method) => this.callCdp(method),
+        );
         await this.callCdp('Runtime.enable');
         await this.callCdp('Debugger.enable');
         await this.bindBreakpoints();
         this.started = true;
         this.startTimeout();
+        await this.callCdp('NodeRuntime.disable');
         await this.callCdp('Runtime.runIfWaitingForDebugger');
       } catch (err) {
         if (err !== kInspectorFailedSentinel) { throw err; }

lib/internal/debugger/inspect_repl.js

@@ -60,6 +60,7 @@ const { fileURLToPath } = require('internal/url');
 const { customInspectSymbol, SideEffectFreeRegExpPrototypeSymbolReplace } = require('internal/util');
 const { inspect: utilInspect } = require('internal/util/inspect');
 const { isObjectLiteral } = require('internal/repl/utils');
+const { waitForDebugger } = require('internal/debugger/inspect_helpers');
 const debuglog = require('internal/util/debuglog').debuglog('inspect');
 
 const SHORTCUTS = {
@@ -1204,9 +1205,13 @@ function createRepl(inspector) {
   }
 
   async function initAfterStart() {
+    const waitForDebuggerOnStart = !!inspector.options?.script;
     waitForInitialBreakRender =
-      !!inspector.options?.script &&
+      waitForDebuggerOnStart &&
       process.env.NODE_INSPECT_RESUME_ON_START !== '1';
+    if (waitForDebuggerOnStart) {
+      await waitForDebugger(inspector.client);
+    }
     await Runtime.enable();
     await Profiler.enable();
     await Profiler.setSamplingInterval({ interval: 100 });
@@ -1215,6 +1220,9 @@ function createRepl(inspector) {
     await Debugger.setBlackboxPatterns({ patterns: [] });
     await Debugger.setPauseOnExceptions({ state: pauseOnExceptionState });
     await restoreBreakpoints();
+    if (waitForDebuggerOnStart) {
+      await inspector.client.callMethod('NodeRuntime.disable');
+    }
     await Runtime.runIfWaitingForDebugger();
     await PromiseResolve();
     waitForInitialBreakRender = false;

test/parallel/test-debugger-run-restart-init.js

@@ -79,9 +79,27 @@ async function assertCommandWaitsForInit(repl, command, gate, calls) {
   const runGate = createGate();
   const restartGate = createGate();
   const gates = [null, runGate, restartGate];
+  const client = new EventEmitter();
+  let nodeRuntimeEnableCount = 0;
+  client.callMethod = async (method) => {
+    calls.push(method);
+    if (method === 'NodeRuntime.enable') {
+      const emitWaiting = () => {
+        calls.push('NodeRuntime.waitingForDebugger');
+        client.emit('NodeRuntime.waitingForDebugger');
+      };
+      // Cover notifications arriving both before and after the enable reply.
+      if (nodeRuntimeEnableCount++ % 2 === 0) {
+        emitWaiting();
+      } else {
+        setImmediate(emitWaiting);
+      }
+    }
+  };
   const inspector = {
-    client: new EventEmitter(),
+    client,
     domainNames: ['Debugger', 'HeapProfiler', 'Profiler', 'Runtime'],
+    options: { script: 'debugger-target.js' },
     stdin: new PassThrough(),
     stdout: new PassThrough(),
     run: common.mustCall(async () => {
@@ -101,6 +119,29 @@ async function assertCommandWaitsForInit(repl, command, gate, calls) {
   await assertCommandWaitsForInit(repl, 'run', runGate, calls);
   await assertCommandWaitsForInit(repl, 'restart', restartGate, calls);
 
+  assert.deepStrictEqual(
+    calls.filter((call) => (
+      call === 'NodeRuntime.enable' ||
+      call === 'NodeRuntime.waitingForDebugger' ||
+      call === 'NodeRuntime.disable' ||
+      call === 'Runtime.runIfWaitingForDebugger'
+    )),
+    [
+      'NodeRuntime.enable',
+      'NodeRuntime.waitingForDebugger',
+      'NodeRuntime.disable',
+      'Runtime.runIfWaitingForDebugger',
+      'NodeRuntime.enable',
+      'NodeRuntime.waitingForDebugger',
+      'NodeRuntime.disable',
+      'Runtime.runIfWaitingForDebugger',
+      'NodeRuntime.enable',
+      'NodeRuntime.waitingForDebugger',
+      'NodeRuntime.disable',
+      'Runtime.runIfWaitingForDebugger',
+    ],
+  );
+
   assert.deepStrictEqual(
     calls.filter((call) => (
       call === 'inspector.run' ||