Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lib/internal/debugger/inspect_helpers.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,9 @@ const {
ArrayPrototypePushApply,
Number,
Promise,
PromiseWithResolvers,
RegExpPrototypeExec,
SafePromiseRace,
StringPrototypeEndsWith,
} = primordials;

Expand All@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
module.exports = {
ensureTrailingNewline,
launchChildProcess,
waitForDebugger,
writeInspectUsageAndExit,
};
6 changes: 6 additions & 0 deletions lib/internal/debugger/inspect_probe.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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; }
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = {
Expand DownExpand Up@@ -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 });
Expand All@@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/debugger.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,9 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
let TIMEOUT = common.platformTimeout(10000);
// Some macOS and Windows machines require more time to receive the outputs from the client.
// https://github.com/nodejs/build/issues/3014
if (common.isWindows || common.isMacOS) {
if (common.isMacOS) {
TIMEOUT = common.platformTimeout(30000);
} else if (common.isWindows) {
Comment on lines +13 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

TIMEOUT = common.platformTimeout(15000);
}

Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-debugger-profile-command.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,18 @@ common.skipIfInspectorDisabled();

const fixtures = require('../common/fixtures');
const startCLI = require('../common/debugger');
const tmpdir = require('../common/tmpdir');

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const cli = startCLI([fixtures.path('debugger/empty.js')]);
tmpdir.refresh();

const rootDir = path.resolve(__dirname, '..', '..');
const cli = startCLI(
[fixtures.path('debugger/empty.js')],
[],
{ cwd: tmpdir.path },
);
Comment on lines +15 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

This comment was marked as spam.

This comment was marked as spam.

@inoway46inoway46Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.


(async () => {
await cli.waitForInitialBreak();
Expand All@@ -25,7 +29,7 @@ const rootDir = path.resolve(__dirname, '..', '..');
await cli.command('profiles[0].save()');
assert.match(cli.output, /Saved profile to .*node\.cpuprofile/);

const cpuprofile = path.resolve(rootDir, 'node.cpuprofile');
const cpuprofile = tmpdir.resolve('node.cpuprofile');
const data = JSON.parse(fs.readFileSync(cpuprofile, 'utf8'));
assert.strictEqual(Array.isArray(data.nodes), true);

Expand Down
43 changes: 42 additions & 1 deletion test/parallel/test-debugger-run-restart-init.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand All@@ -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' ||
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lib/internal/debugger/inspect_helpers.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,9 @@ const {
ArrayPrototypePushApply,
Number,
Promise,
PromiseWithResolvers,
RegExpPrototypeExec,
SafePromiseRace,
StringPrototypeEndsWith,
} = primordials;

Expand All@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
module.exports = {
ensureTrailingNewline,
launchChildProcess,
waitForDebugger,
writeInspectUsageAndExit,
};
6 changes: 6 additions & 0 deletions lib/internal/debugger/inspect_probe.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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; }
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = {
Expand DownExpand Up@@ -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 });
Expand All@@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/debugger.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,9 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
let TIMEOUT = common.platformTimeout(10000);
// Some macOS and Windows machines require more time to receive the outputs from the client.
// https://github.com/nodejs/build/issues/3014
if (common.isWindows || common.isMacOS) {
if (common.isMacOS) {
TIMEOUT = common.platformTimeout(30000);
} else if (common.isWindows) {
Comment on lines +13 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

TIMEOUT = common.platformTimeout(15000);
}

Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-debugger-profile-command.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,18 @@ common.skipIfInspectorDisabled();

const fixtures = require('../common/fixtures');
const startCLI = require('../common/debugger');
const tmpdir = require('../common/tmpdir');

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const cli = startCLI([fixtures.path('debugger/empty.js')]);
tmpdir.refresh();

const rootDir = path.resolve(__dirname, '..', '..');
const cli = startCLI(
[fixtures.path('debugger/empty.js')],
[],
{ cwd: tmpdir.path },
);
Comment on lines +15 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

This comment was marked as spam.

This comment was marked as spam.

@inoway46inoway46Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.


(async () => {
await cli.waitForInitialBreak();
Expand All@@ -25,7 +29,7 @@ const rootDir = path.resolve(__dirname, '..', '..');
await cli.command('profiles[0].save()');
assert.match(cli.output, /Saved profile to .*node\.cpuprofile/);

const cpuprofile = path.resolve(rootDir, 'node.cpuprofile');
const cpuprofile = tmpdir.resolve('node.cpuprofile');
const data = JSON.parse(fs.readFileSync(cpuprofile, 'utf8'));
assert.strictEqual(Array.isArray(data.nodes), true);

Expand Down
43 changes: 42 additions & 1 deletion test/parallel/test-debugger-run-restart-init.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand All@@ -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' ||
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lib/internal/debugger/inspect_helpers.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,9 @@ const {
ArrayPrototypePushApply,
Number,
Promise,
PromiseWithResolvers,
RegExpPrototypeExec,
SafePromiseRace,
StringPrototypeEndsWith,
} = primordials;

Expand All@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
module.exports = {
ensureTrailingNewline,
launchChildProcess,
waitForDebugger,
writeInspectUsageAndExit,
};
6 changes: 6 additions & 0 deletions lib/internal/debugger/inspect_probe.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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; }
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = {
Expand DownExpand Up@@ -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 });
Expand All@@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/debugger.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,9 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
let TIMEOUT = common.platformTimeout(10000);
// Some macOS and Windows machines require more time to receive the outputs from the client.
// https://github.com/nodejs/build/issues/3014
if (common.isWindows || common.isMacOS) {
if (common.isMacOS) {
TIMEOUT = common.platformTimeout(30000);
} else if (common.isWindows) {
Comment on lines +13 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

TIMEOUT = common.platformTimeout(15000);
}

Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-debugger-profile-command.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,18 @@ common.skipIfInspectorDisabled();

const fixtures = require('../common/fixtures');
const startCLI = require('../common/debugger');
const tmpdir = require('../common/tmpdir');

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const cli = startCLI([fixtures.path('debugger/empty.js')]);
tmpdir.refresh();

const rootDir = path.resolve(__dirname, '..', '..');
const cli = startCLI(
[fixtures.path('debugger/empty.js')],
[],
{ cwd: tmpdir.path },
);
Comment on lines +15 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

This comment was marked as spam.

This comment was marked as spam.

@inoway46inoway46Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.


(async () => {
await cli.waitForInitialBreak();
Expand All@@ -25,7 +29,7 @@ const rootDir = path.resolve(__dirname, '..', '..');
await cli.command('profiles[0].save()');
assert.match(cli.output, /Saved profile to .*node\.cpuprofile/);

const cpuprofile = path.resolve(rootDir, 'node.cpuprofile');
const cpuprofile = tmpdir.resolve('node.cpuprofile');
const data = JSON.parse(fs.readFileSync(cpuprofile, 'utf8'));
assert.strictEqual(Array.isArray(data.nodes), true);

Expand Down
43 changes: 42 additions & 1 deletion test/parallel/test-debugger-run-restart-init.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand All@@ -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' ||
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lib/internal/debugger/inspect_helpers.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,9 @@ const {
ArrayPrototypePushApply,
Number,
Promise,
PromiseWithResolvers,
RegExpPrototypeExec,
SafePromiseRace,
StringPrototypeEndsWith,
} = primordials;

Expand All@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
module.exports = {
ensureTrailingNewline,
launchChildProcess,
waitForDebugger,
writeInspectUsageAndExit,
};
6 changes: 6 additions & 0 deletions lib/internal/debugger/inspect_probe.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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; }
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = {
Expand DownExpand Up@@ -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 });
Expand All@@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/debugger.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,9 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
let TIMEOUT = common.platformTimeout(10000);
// Some macOS and Windows machines require more time to receive the outputs from the client.
// https://github.com/nodejs/build/issues/3014
if (common.isWindows || common.isMacOS) {
if (common.isMacOS) {
TIMEOUT = common.platformTimeout(30000);
} else if (common.isWindows) {
Comment on lines +13 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

TIMEOUT = common.platformTimeout(15000);
}

Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-debugger-profile-command.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,18 @@ common.skipIfInspectorDisabled();

const fixtures = require('../common/fixtures');
const startCLI = require('../common/debugger');
const tmpdir = require('../common/tmpdir');

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const cli = startCLI([fixtures.path('debugger/empty.js')]);
tmpdir.refresh();

const rootDir = path.resolve(__dirname, '..', '..');
const cli = startCLI(
[fixtures.path('debugger/empty.js')],
[],
{ cwd: tmpdir.path },
);
Comment on lines +15 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

This comment was marked as spam.

This comment was marked as spam.

@inoway46inoway46Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.


(async () => {
await cli.waitForInitialBreak();
Expand All@@ -25,7 +29,7 @@ const rootDir = path.resolve(__dirname, '..', '..');
await cli.command('profiles[0].save()');
assert.match(cli.output, /Saved profile to .*node\.cpuprofile/);

const cpuprofile = path.resolve(rootDir, 'node.cpuprofile');
const cpuprofile = tmpdir.resolve('node.cpuprofile');
const data = JSON.parse(fs.readFileSync(cpuprofile, 'utf8'));
assert.strictEqual(Array.isArray(data.nodes), true);

Expand Down
43 changes: 42 additions & 1 deletion test/parallel/test-debugger-run-restart-init.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand All@@ -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' ||
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lib/internal/debugger/inspect_helpers.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,9 @@ const {
ArrayPrototypePushApply,
Number,
Promise,
PromiseWithResolvers,
RegExpPrototypeExec,
SafePromiseRace,
StringPrototypeEndsWith,
} = primordials;

Expand All@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
module.exports = {
ensureTrailingNewline,
launchChildProcess,
waitForDebugger,
writeInspectUsageAndExit,
};
6 changes: 6 additions & 0 deletions lib/internal/debugger/inspect_probe.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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; }
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = {
Expand DownExpand Up@@ -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 });
Expand All@@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/debugger.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,9 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
let TIMEOUT = common.platformTimeout(10000);
// Some macOS and Windows machines require more time to receive the outputs from the client.
// https://github.com/nodejs/build/issues/3014
if (common.isWindows || common.isMacOS) {
if (common.isMacOS) {
TIMEOUT = common.platformTimeout(30000);
} else if (common.isWindows) {
Comment on lines +13 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

TIMEOUT = common.platformTimeout(15000);
}

Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-debugger-profile-command.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,18 @@ common.skipIfInspectorDisabled();

const fixtures = require('../common/fixtures');
const startCLI = require('../common/debugger');
const tmpdir = require('../common/tmpdir');

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const cli = startCLI([fixtures.path('debugger/empty.js')]);
tmpdir.refresh();

const rootDir = path.resolve(__dirname, '..', '..');
const cli = startCLI(
[fixtures.path('debugger/empty.js')],
[],
{ cwd: tmpdir.path },
);
Comment on lines +15 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

This comment was marked as spam.

This comment was marked as spam.

@inoway46inoway46Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.


(async () => {
await cli.waitForInitialBreak();
Expand All@@ -25,7 +29,7 @@ const rootDir = path.resolve(__dirname, '..', '..');
await cli.command('profiles[0].save()');
assert.match(cli.output, /Saved profile to .*node\.cpuprofile/);

const cpuprofile = path.resolve(rootDir, 'node.cpuprofile');
const cpuprofile = tmpdir.resolve('node.cpuprofile');
const data = JSON.parse(fs.readFileSync(cpuprofile, 'utf8'));
assert.strictEqual(Array.isArray(data.nodes), true);

Expand Down
43 changes: 42 additions & 1 deletion test/parallel/test-debugger-run-restart-init.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand All@@ -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' ||
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lib/internal/debugger/inspect_helpers.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,9 @@ const {
ArrayPrototypePushApply,
Number,
Promise,
PromiseWithResolvers,
RegExpPrototypeExec,
SafePromiseRace,
StringPrototypeEndsWith,
} = primordials;

Expand All@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
module.exports = {
ensureTrailingNewline,
launchChildProcess,
waitForDebugger,
writeInspectUsageAndExit,
};
6 changes: 6 additions & 0 deletions lib/internal/debugger/inspect_probe.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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; }
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = {
Expand DownExpand Up@@ -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 });
Expand All@@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/debugger.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,9 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
let TIMEOUT = common.platformTimeout(10000);
// Some macOS and Windows machines require more time to receive the outputs from the client.
// https://github.com/nodejs/build/issues/3014
if (common.isWindows || common.isMacOS) {
if (common.isMacOS) {
TIMEOUT = common.platformTimeout(30000);
} else if (common.isWindows) {
Comment on lines +13 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

TIMEOUT = common.platformTimeout(15000);
}

Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-debugger-profile-command.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,18 @@ common.skipIfInspectorDisabled();

const fixtures = require('../common/fixtures');
const startCLI = require('../common/debugger');
const tmpdir = require('../common/tmpdir');

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const cli = startCLI([fixtures.path('debugger/empty.js')]);
tmpdir.refresh();

const rootDir = path.resolve(__dirname, '..', '..');
const cli = startCLI(
[fixtures.path('debugger/empty.js')],
[],
{ cwd: tmpdir.path },
);
Comment on lines +15 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

This comment was marked as spam.

This comment was marked as spam.

@inoway46inoway46Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.


(async () => {
await cli.waitForInitialBreak();
Expand All@@ -25,7 +29,7 @@ const rootDir = path.resolve(__dirname, '..', '..');
await cli.command('profiles[0].save()');
assert.match(cli.output, /Saved profile to .*node\.cpuprofile/);

const cpuprofile = path.resolve(rootDir, 'node.cpuprofile');
const cpuprofile = tmpdir.resolve('node.cpuprofile');
const data = JSON.parse(fs.readFileSync(cpuprofile, 'utf8'));
assert.strictEqual(Array.isArray(data.nodes), true);

Expand Down
43 changes: 42 additions & 1 deletion test/parallel/test-debugger-run-restart-init.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand All@@ -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' ||
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lib/internal/debugger/inspect_helpers.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,9 @@ const {
ArrayPrototypePushApply,
Number,
Promise,
PromiseWithResolvers,
RegExpPrototypeExec,
SafePromiseRace,
StringPrototypeEndsWith,
} = primordials;

Expand All@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
module.exports = {
ensureTrailingNewline,
launchChildProcess,
waitForDebugger,
writeInspectUsageAndExit,
};
6 changes: 6 additions & 0 deletions lib/internal/debugger/inspect_probe.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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; }
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = {
Expand DownExpand Up@@ -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 });
Expand All@@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/debugger.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,9 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
let TIMEOUT = common.platformTimeout(10000);
// Some macOS and Windows machines require more time to receive the outputs from the client.
// https://github.com/nodejs/build/issues/3014
if (common.isWindows || common.isMacOS) {
if (common.isMacOS) {
TIMEOUT = common.platformTimeout(30000);
} else if (common.isWindows) {
Comment on lines +13 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

TIMEOUT = common.platformTimeout(15000);
}

Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-debugger-profile-command.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,18 @@ common.skipIfInspectorDisabled();

const fixtures = require('../common/fixtures');
const startCLI = require('../common/debugger');
const tmpdir = require('../common/tmpdir');

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const cli = startCLI([fixtures.path('debugger/empty.js')]);
tmpdir.refresh();

const rootDir = path.resolve(__dirname, '..', '..');
const cli = startCLI(
[fixtures.path('debugger/empty.js')],
[],
{ cwd: tmpdir.path },
);
Comment on lines +15 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

This comment was marked as spam.

This comment was marked as spam.

@inoway46inoway46Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.


(async () => {
await cli.waitForInitialBreak();
Expand All@@ -25,7 +29,7 @@ const rootDir = path.resolve(__dirname, '..', '..');
await cli.command('profiles[0].save()');
assert.match(cli.output, /Saved profile to .*node\.cpuprofile/);

const cpuprofile = path.resolve(rootDir, 'node.cpuprofile');
const cpuprofile = tmpdir.resolve('node.cpuprofile');
const data = JSON.parse(fs.readFileSync(cpuprofile, 'utf8'));
assert.strictEqual(Array.isArray(data.nodes), true);

Expand Down
43 changes: 42 additions & 1 deletion test/parallel/test-debugger-run-restart-init.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand All@@ -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' ||
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lib/internal/debugger/inspect_helpers.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,9 @@ const {
ArrayPrototypePushApply,
Number,
Promise,
PromiseWithResolvers,
RegExpPrototypeExec,
SafePromiseRace,
StringPrototypeEndsWith,
} = primordials;

Expand All@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -189,5 +229,6 @@ async function launchChildProcess(childArgs, inspectHost, inspectPort,
module.exports = {
ensureTrailingNewline,
launchChildProcess,
waitForDebugger,
writeInspectUsageAndExit,
};
6 changes: 6 additions & 0 deletions lib/internal/debugger/inspect_probe.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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; }
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/debugger/inspect_repl.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 = {
Expand DownExpand Up@@ -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 });
Expand All@@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/common/debugger.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,9 @@ const BREAK_MESSAGE = new RegExp('(?:' + [
let TIMEOUT = common.platformTimeout(10000);
// Some macOS and Windows machines require more time to receive the outputs from the client.
// https://github.com/nodejs/build/issues/3014
if (common.isWindows || common.isMacOS) {
if (common.isMacOS) {
TIMEOUT = common.platformTimeout(30000);
} else if (common.isWindows) {
Comment on lines +13 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon

Thanks for working on this!

Could you clarify why the timeout needs to be increased? If waitForDebugger() addresses the race condition, I would expect the timeout increase to be unnecessary.

Also, I think keeping the original timeout would make the stress test results more convincing, since otherwise it’s difficult to tell whether the improvement comes from waitForDebugger() or simply from the increased timeout.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks for clarifying. If Debugger.paused was emitted but not received by the JavaScript layer within 15 seconds, could you share a failing run or trace showing that?

Because the current stress runs include both changes, it is difficult to tell whether waitForDebugger() alone resolves the issue.

TIMEOUT = common.platformTimeout(15000);
}

Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-debugger-profile-command.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,18 @@ common.skipIfInspectorDisabled();

const fixtures = require('../common/fixtures');
const startCLI = require('../common/debugger');
const tmpdir = require('../common/tmpdir');

const assert = require('assert');
const fs = require('fs');
const path = require('path');

const cli = startCLI([fixtures.path('debugger/empty.js')]);
tmpdir.refresh();

const rootDir = path.resolve(__dirname, '..', '..');
const cli = startCLI(
[fixtures.path('debugger/empty.js')],
[],
{ cwd: tmpdir.path },
);
Comment on lines +15 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Could you remove this change from the PR?

It appears to be needed only for the --repeat 1000 -j16 stress run, where concurrent instances of this test share the same node.cpuprofile. It is unrelated to the debugger startup race and is not needed in the normal CI run.

If it is needed for stress validation, it can remain only in the stress-test branch or workflow.

This comment was marked as spam.

This comment was marked as spam.

@inoway46inoway46Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Archkon
Thanks. My concern is the PR scope, not the -j4 setting.

The linked run still executes this test 1000 times. Normal CI executes it only once, so the node.cpuprofile collision shown there is specific to the stress run. I think this change should be removed from this PR and kept only in the stress-test setup if needed.

Also, please leave this thread unresolved while this concern is still under discussion.


(async () => {
await cli.waitForInitialBreak();
Expand All@@ -25,7 +29,7 @@ const rootDir = path.resolve(__dirname, '..', '..');
await cli.command('profiles[0].save()');
assert.match(cli.output, /Saved profile to .*node\.cpuprofile/);

const cpuprofile = path.resolve(rootDir, 'node.cpuprofile');
const cpuprofile = tmpdir.resolve('node.cpuprofile');
const data = JSON.parse(fs.readFileSync(cpuprofile, 'utf8'));
assert.strictEqual(Array.isArray(data.nodes), true);

Expand Down
43 changes: 42 additions & 1 deletion test/parallel/test-debugger-run-restart-init.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand All@@ -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' ||
Expand Down
Loading