Skip to content

Commit 821e267

Browse files
panvaArchkon
panva
and
Archkon
committed
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>
1 parent 404b0cf commit 821e267

5 files changed

Lines changed: 269 additions & 3 deletions

File tree

‎lib/internal/debugger/inspect_helpers.js‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ const {
44
ArrayPrototypePushApply,
55
Number,
66
Promise,
7+
PromiseWithResolvers,
78
RegExpPrototypeExec,
9+
SafePromiseRace,
810
StringPrototypeEndsWith,
911
} = primordials;
1012

@@ -18,7 +20,10 @@ const {
1820
AbortController,
1921
} = require('internal/abort_controller');
2022

21-
const { ERR_DEBUGGER_STARTUP_ERROR } = require('internal/errors').codes;
23+
const {
24+
ERR_DEBUGGER_ERROR,
25+
ERR_DEBUGGER_STARTUP_ERROR,
26+
} = require('internal/errors').codes;
2227
const {
2328
exitCodes: {
2429
kInvalidCommandLineArgument,
@@ -61,6 +66,48 @@ function ensureTrailingNewline(text) {
6166
return StringPrototypeEndsWith(text, '\n') ? text : `${text}\n`;
6267
}
6368

69+
async function waitForDebugger(
70+
client,
71+
callMethod = (method) => client.callMethod(method),
72+
) {
73+
const {
74+
promise: waitingPromise,
75+
resolve: resolveWaiting,
76+
} = PromiseWithResolvers();
77+
const {
78+
promise: closedPromise,
79+
reject: rejectClosed,
80+
} = PromiseWithResolvers();
81+
const onWaiting = () => resolveWaiting();
82+
const onClose = () => {
83+
rejectClosed(new ERR_DEBUGGER_ERROR(
84+
'Debugger session ended while waiting for target startup'));
85+
};
86+
87+
// The inspector can accept a connection before the target reaches its
88+
// startup wait. Enabling NodeRuntime makes that state observable whether
89+
// the target was already waiting or starts waiting later.
90+
client.once('NodeRuntime.waitingForDebugger', onWaiting);
91+
client.once('close', onClose);
92+
try {
93+
await SafePromiseRace([
94+
callMethod('NodeRuntime.enable'),
95+
closedPromise,
96+
]);
97+
await SafePromiseRace([
98+
waitingPromise,
99+
closedPromise,
100+
]);
101+
await SafePromiseRace([
102+
callMethod('NodeRuntime.disable'),
103+
closedPromise,
104+
]);
105+
} finally {
106+
client.removeListener('NodeRuntime.waitingForDebugger', onWaiting);
107+
client.removeListener('close', onClose);
108+
}
109+
}
110+
64111
function writeInspectUsageAndExit(invokedAs, message, exitCode) {
65112
const code = exitCode ?? (message ? kInvalidCommandLineArgument : 0);
66113
const out = code === 0 ? process.stdout : process.stderr;
@@ -189,5 +236,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
189236
module.exports = {
190237
ensureTrailingNewline,
191238
launchChildProcess,
239+
waitForDebugger,
192240
writeInspectUsageAndExit,
193241
};

‎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 });

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,29 @@ async function assertCommandWaitsForInit(repl, command, gate, calls) {
7979
const runGate = createGate();
8080
const restartGate = createGate();
8181
const gates = [null, runGate, restartGate];
82+
const client = new EventEmitter();
83+
let nodeRuntimeEnableCount = 0;
84+
client.callMethod = common.mustCall(async (method) => {
85+
calls.push(method);
86+
if (method === 'NodeRuntime.enable') {
87+
const emitWaiting = () => {
88+
calls.push('NodeRuntime.waitingForDebugger');
89+
client.emit('NodeRuntime.waitingForDebugger');
90+
};
91+
// Cover notifications arriving both before and after the enable reply.
92+
if (nodeRuntimeEnableCount++ % 2 === 0) {
93+
emitWaiting();
94+
} else {
95+
setImmediate(emitWaiting);
96+
}
97+
} else {
98+
assert.strictEqual(method, 'NodeRuntime.disable');
99+
}
100+
}, 6);
82101
const inspector = {
83-
client: new EventEmitter(),
102+
client,
84103
domainNames: ['Debugger', 'HeapProfiler', 'Profiler', 'Runtime'],
104+
options: { script: 'debugger-target.js' },
85105
stdin: new PassThrough(),
86106
stdout: new PassThrough(),
87107
run: common.mustCall(async () => {
@@ -101,6 +121,29 @@ async function assertCommandWaitsForInit(repl, command, gate, calls) {
101121
await assertCommandWaitsForInit(repl, 'run', runGate, calls);
102122
await assertCommandWaitsForInit(repl, 'restart', restartGate, calls);
103123

124+
assert.deepStrictEqual(
125+
calls.filter((call) => (
126+
call === 'NodeRuntime.enable' ||
127+
call === 'NodeRuntime.waitingForDebugger' ||
128+
call === 'NodeRuntime.disable' ||
129+
call === 'Runtime.runIfWaitingForDebugger'
130+
)),
131+
[
132+
'NodeRuntime.enable',
133+
'NodeRuntime.waitingForDebugger',
134+
'NodeRuntime.disable',
135+
'Runtime.runIfWaitingForDebugger',
136+
'NodeRuntime.enable',
137+
'NodeRuntime.waitingForDebugger',
138+
'NodeRuntime.disable',
139+
'Runtime.runIfWaitingForDebugger',
140+
'NodeRuntime.enable',
141+
'NodeRuntime.waitingForDebugger',
142+
'NodeRuntime.disable',
143+
'Runtime.runIfWaitingForDebugger',
144+
],
145+
);
146+
104147
assert.deepStrictEqual(
105148
calls.filter((call) => (
106149
call === 'inspector.run' ||
@@ -116,4 +159,25 @@ async function assertCommandWaitsForInit(repl, command, gate, calls) {
116159
);
117160

118161
repl.close();
162+
163+
const attachCalls = [];
164+
const attachClient = new EventEmitter();
165+
attachClient.callMethod = common.mustNotCall();
166+
const attachInspector = {
167+
client: attachClient,
168+
domainNames: ['Debugger', 'HeapProfiler', 'Profiler', 'Runtime'],
169+
options: {},
170+
stdin: new PassThrough(),
171+
stdout: new PassThrough(),
172+
suspendReplWhile(fn) {
173+
return fn();
174+
},
175+
};
176+
177+
for (const domain of attachInspector.domainNames) {
178+
attachInspector[domain] = createAgent(domain, attachCalls, []);
179+
}
180+
181+
const attachRepl = await createRepl(attachInspector)();
182+
attachRepl.close();
119183
})().then(common.mustCall());

‎test/parallel/test-debugger-wait-for-debugger.js‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
const common = require('../common');
5+
6+
common.skipIfInspectorDisabled();
7+
8+
const assert = require('assert');
9+
const { EventEmitter } = require('events');
10+
const {
11+
waitForDebugger,
12+
} = require('internal/debugger/inspect_helpers');
13+
14+
function assertListenersRemoved(client) {
15+
assert.strictEqual(
16+
client.listenerCount('NodeRuntime.waitingForDebugger'),
17+
0,
18+
);
19+
assert.strictEqual(client.listenerCount('close'), 0);
20+
}
21+
22+
async function testWaitingNotification(beforeEnableReply) {
23+
const client = new EventEmitter();
24+
const calls = [];
25+
client.callMethod = common.mustCall(async (method) => {
26+
calls.push(method);
27+
const emitWaiting = () => {
28+
client.emit('NodeRuntime.waitingForDebugger');
29+
};
30+
if (method === 'NodeRuntime.enable') {
31+
if (beforeEnableReply) {
32+
emitWaiting();
33+
} else {
34+
setImmediate(emitWaiting);
35+
}
36+
} else {
37+
assert.strictEqual(method, 'NodeRuntime.disable');
38+
}
39+
}, 2);
40+
41+
await waitForDebugger(client);
42+
assert.deepStrictEqual(calls, [
43+
'NodeRuntime.enable',
44+
'NodeRuntime.disable',
45+
]);
46+
assertListenersRemoved(client);
47+
}
48+
49+
async function testCloseWhileWaiting(beforeEnableReply) {
50+
const client = new EventEmitter();
51+
client.callMethod = common.mustCall((method) => {
52+
assert.strictEqual(method, 'NodeRuntime.enable');
53+
setImmediate(() => client.emit('close'));
54+
return beforeEnableReply ? new Promise(() => {}) : Promise.resolve();
55+
});
56+
57+
await assert.rejects(
58+
waitForDebugger(client),
59+
{
60+
code: 'ERR_DEBUGGER_ERROR',
61+
message: 'Debugger session ended while waiting for target startup',
62+
},
63+
);
64+
assertListenersRemoved(client);
65+
}
66+
67+
async function testCloseWhileDisabling() {
68+
const client = new EventEmitter();
69+
client.callMethod = common.mustCall((method) => {
70+
if (method === 'NodeRuntime.enable') {
71+
client.emit('NodeRuntime.waitingForDebugger');
72+
return Promise.resolve();
73+
}
74+
assert.strictEqual(method, 'NodeRuntime.disable');
75+
setImmediate(() => client.emit('close'));
76+
return new Promise(() => {});
77+
}, 2);
78+
79+
await assert.rejects(
80+
waitForDebugger(client),
81+
{
82+
code: 'ERR_DEBUGGER_ERROR',
83+
message: 'Debugger session ended while waiting for target startup',
84+
},
85+
);
86+
assertListenersRemoved(client);
87+
}
88+
89+
async function testEnableFailure() {
90+
const client = new EventEmitter();
91+
const expected = new Error('NodeRuntime.enable failed');
92+
client.callMethod = common.mustCall(async (method) => {
93+
assert.strictEqual(method, 'NodeRuntime.enable');
94+
throw expected;
95+
});
96+
97+
await assert.rejects(
98+
waitForDebugger(client),
99+
(error) => {
100+
assert.strictEqual(error, expected);
101+
return true;
102+
},
103+
);
104+
assertListenersRemoved(client);
105+
}
106+
107+
async function testDisableFailure() {
108+
const client = new EventEmitter();
109+
const expected = new Error('NodeRuntime.disable failed');
110+
client.callMethod = common.mustCall(async (method) => {
111+
if (method === 'NodeRuntime.enable') {
112+
client.emit('NodeRuntime.waitingForDebugger');
113+
return;
114+
}
115+
assert.strictEqual(method, 'NodeRuntime.disable');
116+
throw expected;
117+
}, 2);
118+
119+
await assert.rejects(
120+
waitForDebugger(client),
121+
(error) => {
122+
assert.strictEqual(error, expected);
123+
return true;
124+
},
125+
);
126+
assertListenersRemoved(client);
127+
}
128+
129+
(async () => {
130+
await testWaitingNotification(true);
131+
await testWaitingNotification(false);
132+
await testCloseWhileWaiting(true);
133+
await testCloseWhileWaiting(false);
134+
await testCloseWhileDisabling();
135+
await testEnableFailure();
136+
await testDisableFailure();
137+
})().then(common.mustCall());

0 commit comments

Comments
 (0)