Skip to content

Commit 2004fbd

Browse files
panvaArchkon
andcommitted
debugger: wait for target startup
The inspector can accept a connection before an --inspect-brk target enters its frontend wait. Runtime.runIfWaitingForDebugger can then be handled too early, allowing the target to subsequently block forever. Wait for NodeRuntime.waitingForDebugger before initializing and releasing launched targets. Race the handshake against disconnects and apply it to both interactive and probe startup. Refs: #64116 Assisted-by: codex:gpt-5.6-sol Co-authored-by: Archkon <180910180+Archkon@users.noreply.github.com> Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com> Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65194 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 671ff97 commit 2004fbd

7 files changed

Lines changed: 629 additions & 3 deletions

lib/internal/debugger/inspect_helpers.js

Lines changed: 195 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,20 @@
11
'use strict';
22

33
const {
4+
ArrayPrototypePop,
5+
ArrayPrototypePush,
46
ArrayPrototypePushApply,
7+
MapPrototypeGet,
58
Number,
69
Promise,
10+
PromiseWithResolvers,
711
RegExpPrototypeExec,
12+
RegExpPrototypeSymbolReplace,
13+
SafePromiseRace,
814
StringPrototypeEndsWith,
15+
StringPrototypeIndexOf,
16+
StringPrototypeSlice,
17+
StringPrototypeStartsWith,
918
} = primordials;
1019

1120
const { spawn } = require('child_process');
@@ -18,12 +27,24 @@ const {
1827
AbortController,
1928
} = require('internal/abort_controller');
2029

21-
const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes;
30+
const {
31+
ERR_DEBUGGER_ERROR,
32+
ERR_DEBUGGER_STARTUP_ERROR,
33+
} = require('internal/errors').codes;
2234
const {
2335
exitCodes: {
2436
kInvalidCommandLineArgument,
2537
},
2638
} = internalBinding('errors');
39+
const {
40+
types: {
41+
kBoolean,
42+
kNoOp,
43+
kV8Option,
44+
},
45+
} = internalBinding('options');
46+
47+
const { getCLIOptionsInfo } = require('internal/options');
2748

2849
const debugRegex = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\//;
2950

@@ -61,6 +82,176 @@ function ensureTrailingNewline(text) {
6182
return StringPrototypeEndsWith(text, '\n') ? text : `${text}\n`;
6283
}
6384

85+
// Mirror OptionsParser::Parse() far enough to find the child script. Options
86+
// before it must not undo the inspector setup added by launchChildProcess().
87+
function validateChildArgs(childArgs) {
88+
const { options, aliases } = getCLIOptionsInfo();
89+
const syntheticArgs = [];
90+
let breakFirstLine = true;
91+
let childArgIndex = 0;
92+
let inspectorEnabled = true;
93+
94+
function peekArg() {
95+
return syntheticArgs.length > 0 ?
96+
syntheticArgs[syntheticArgs.length - 1] :
97+
childArgs[childArgIndex];
98+
}
99+
100+
function shiftArg() {
101+
return syntheticArgs.length > 0 ?
102+
ArrayPrototypePop(syntheticArgs) :
103+
childArgs[childArgIndex++];
104+
}
105+
106+
while (true) {
107+
const nextArg = peekArg();
108+
if (nextArg === undefined || nextArg.length <= 1 || nextArg[0] !== '-') {
109+
break;
110+
}
111+
112+
const isSynthetic = syntheticArgs.length > 0;
113+
const arg = shiftArg();
114+
if (arg === '--') { break; }
115+
if (!isSynthetic &&
116+
(arg === '--experimental-config-file' ||
117+
arg === '--experimental-default-config-file')) {
118+
// ConfigReader rewrites these to an inline default path before parsing.
119+
continue;
120+
}
121+
if (!isSynthetic &&
122+
StringPrototypeStartsWith(
123+
arg, '--experimental-default-config-file=')) {
124+
// ConfigReader rejects this form before parsing the remaining options.
125+
return;
126+
}
127+
128+
const equalsIndex = arg[1] === '-' ? StringPrototypeIndexOf(arg, '=') : -1;
129+
let name = equalsIndex === -1 ? arg : StringPrototypeSlice(arg, 0, equalsIndex);
130+
if (name.length > 2) {
131+
name = `${StringPrototypeSlice(name, 0, 2)}${
132+
RegExpPrototypeSymbolReplace(/_/g, StringPrototypeSlice(name, 2), '-')}`;
133+
}
134+
135+
let isNegation = false;
136+
if (StringPrototypeStartsWith(name, '--no-')) {
137+
name = `--${StringPrototypeSlice(name, 5)}`;
138+
isNegation = true;
139+
}
140+
141+
while (true) {
142+
let expansion = MapPrototypeGet(aliases, name);
143+
if (expansion === undefined && equalsIndex !== -1) {
144+
expansion = MapPrototypeGet(aliases, `${name}=`);
145+
}
146+
const aliasArg = peekArg();
147+
if (expansion === undefined &&
148+
aliasArg !== undefined &&
149+
aliasArg.length > 0 &&
150+
aliasArg[0] !== '-') {
151+
expansion = MapPrototypeGet(aliases, `${name} <arg>`);
152+
}
153+
if (expansion === undefined) { break; }
154+
155+
const previousName = name;
156+
// process.allowedNodeEnvironmentFlags may remove a self-recursive
157+
// first entry from the cached alias metadata. Preserve the native
158+
// parser's synthetic option terminator in that case.
159+
if (expansion[0] === '--') {
160+
for (let i = expansion.length - 1; i >= 0; i--) {
161+
ArrayPrototypePush(syntheticArgs, expansion[i]);
162+
}
163+
break;
164+
}
165+
name = expansion[0];
166+
for (let i = expansion.length - 1; i > 0; i--) {
167+
ArrayPrototypePush(syntheticArgs, expansion[i]);
168+
}
169+
if (name === previousName) { break; }
170+
}
171+
172+
const info = MapPrototypeGet(options, name);
173+
if (info === undefined) { continue; }
174+
if (isNegation && info.type !== kBoolean && info.type !== kV8Option) {
175+
return;
176+
}
177+
if (info.type === kBoolean || info.type === kNoOp || info.type === kV8Option) {
178+
if (name === '--inspect') {
179+
inspectorEnabled = !isNegation;
180+
} else if (name === '--inspect-brk') {
181+
breakFirstLine = !isNegation;
182+
if (!isNegation) { inspectorEnabled = true; }
183+
} else if (!isNegation &&
184+
(name === '--inspect-wait' ||
185+
name === '--inspect-brk-node')) {
186+
inspectorEnabled = true;
187+
}
188+
continue;
189+
}
190+
191+
if (equalsIndex !== -1) {
192+
if (equalsIndex === arg.length - 1) { return; }
193+
continue;
194+
}
195+
196+
const value = peekArg();
197+
if (value === undefined || (value.length > 0 && value[0] === '-')) {
198+
return;
199+
}
200+
shiftArg();
201+
}
202+
203+
if (!inspectorEnabled) {
204+
throw new ERR_DEBUGGER_STARTUP_ERROR(
205+
'--no-inspect is incompatible with node inspect before the child script');
206+
}
207+
if (!breakFirstLine) {
208+
throw new ERR_DEBUGGER_STARTUP_ERROR(
209+
'--no-inspect-brk is incompatible with node inspect before the child script');
210+
}
211+
}
212+
213+
async function waitForDebugger(
214+
client,
215+
callMethod = (method) => client.callMethod(method),
216+
) {
217+
const {
218+
promise: waitingPromise,
219+
resolve: resolveWaiting,
220+
} = PromiseWithResolvers();
221+
const {
222+
promise: closedPromise,
223+
reject: rejectClosed,
224+
} = PromiseWithResolvers();
225+
const onWaiting = () => resolveWaiting();
226+
const onClose = () => {
227+
rejectClosed(new ERR_DEBUGGER_ERROR(
228+
'Debugger session ended while waiting for target startup'));
229+
};
230+
231+
// The inspector can accept a connection before the target reaches its
232+
// startup wait. Enabling NodeRuntime makes that state observable whether
233+
// the target was already waiting or starts waiting later.
234+
client.once('NodeRuntime.waitingForDebugger', onWaiting);
235+
client.once('close', onClose);
236+
try {
237+
await SafePromiseRace([
238+
callMethod('NodeRuntime.enable'),
239+
closedPromise,
240+
]);
241+
await SafePromiseRace([
242+
waitingPromise,
243+
closedPromise,
244+
]);
245+
await SafePromiseRace([
246+
callMethod('NodeRuntime.disable'),
247+
closedPromise,
248+
]);
249+
} finally {
250+
client.removeListener('NodeRuntime.waitingForDebugger', onWaiting);
251+
client.removeListener('close', onClose);
252+
}
253+
}
254+
64255
function writeInspectUsageAndExit(invokedAs, message, exitCode) {
65256
const code = exitCode ?? (message ? kInvalidCommandLineArgument : 0);
66257
const out = code === 0 ? process.stdout : process.stderr;
@@ -141,6 +332,8 @@ probe output schema.
141332

142333
async function launchChildProcess(childArgs, inspectHost, inspectPort,
143334
childOutput, options = { __proto__: null }) {
335+
validateChildArgs(childArgs);
336+
144337
if (!options.skipPortPreflight) {
145338
await portIsFree(inspectHost, inspectPort);
146339
}
@@ -189,5 +382,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
189382
module.exports = {
190383
ensureTrailingNewline,
191384
launchChildProcess,
385+
waitForDebugger,
192386
writeInspectUsageAndExit,
193387
};

lib/internal/debugger/inspect_probe.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const InspectClient = require('internal/debugger/inspect_client');
3333
const {
3434
ensureTrailingNewline,
3535
launchChildProcess,
36+
waitForDebugger,
3637
} = require('internal/debugger/inspect_helpers');
3738

3839
const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes;
@@ -1044,6 +1045,17 @@ class ProbeInspectorSession {
10441045
this.connected = true;
10451046

10461047
try {
1048+
try {
1049+
await waitForDebugger(
1050+
this.client,
1051+
(method) => this.callCdp(method),
1052+
);
1053+
} catch (err) {
1054+
// A close event may have completed the structured report while the
1055+
// readiness helper was rejecting its disconnect race.
1056+
if (this.finished) { throw kInspectorFailedSentinel; }
1057+
throw err;
1058+
}
10471059
await this.callCdp('Runtime.enable');
10481060
await this.callCdp('Debugger.enable');
10491061
await this.bindBreakpoints();

lib/internal/debugger/inspect_repl.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const { fileURLToPath } = require('internal/url');
6060
const { customInspectSymbol, SideEffectFreeRegExpPrototypeSymbolReplace } = require('internal/util');
6161
const { inspect: utilInspect } = require('internal/util/inspect');
6262
const { isObjectLiteral } = require('internal/repl/utils');
63+
const { waitForDebugger } = require('internal/debugger/inspect_helpers');
6364
const debuglog = require('internal/util/debuglog').debuglog('inspect');
6465

6566
const SHORTCUTS = {
@@ -1204,9 +1205,13 @@ function createRepl(inspector) {
12041205
}
12051206

12061207
async function initAfterStart() {
1208+
const waitForDebuggerOnStart = !!inspector.options?.script;
12071209
waitForInitialBreakRender =
1208-
!!inspector.options?.script &&
1210+
waitForDebuggerOnStart &&
12091211
process.env.NODE_INSPECT_RESUME_ON_START !== '1';
1212+
if (waitForDebuggerOnStart) {
1213+
await waitForDebugger(inspector.client);
1214+
}
12101215
await Runtime.enable();
12111216
await Profiler.enable();
12121217
await Profiler.setSamplingInterval({ interval: 100 });

0 commit comments

Comments
 (0)