f2aaf63846This test randomly times out (~120s) on CI due to a race condition between child-process restart (triggered by touching the watched file) and the second inspector-session connection. The old code used an interval-based restart (write every 500ms) and a 'gettingDebuggedPid' flag to pause writes during a session. This still left a race window where getDebuggedPid() would attempt to connect the inspector via HTTP GET /json/list + WebSocket upgrade either before the new child was ready (empty target list) or after the old session was being destroyed, causing the promise to hang. Fix: Replace the interval with a single write that triggers exactly one restart, then wait for the restarted child's 'safe to debug now' stdout line before connecting the second inspector session. This eliminates the race by ensuring the new child process and its inspector session are fully ready before any connection attempt. Removes the now-unused gettingDebuggedPid flag and the pending setTimeout delay that was needed as a backstop for the interval. Fixes: https://github.com/nodejs/node/issues/44898 Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: https://github.com/nodejs/node/pull/63361 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
90 lines
3.2 KiB
JavaScript
90 lines
3.2 KiB
JavaScript
import * as common from '../common/index.mjs';
|
|
import * as fixtures from '../common/fixtures.mjs';
|
|
import assert from 'node:assert';
|
|
import { describe, it } from 'node:test';
|
|
import { writeFileSync, readFileSync } from 'node:fs';
|
|
import { NodeInstance } from '../common/inspector-helper.js';
|
|
|
|
|
|
if (common.isIBMi)
|
|
common.skip('IBMi does not support `fs.watch()`');
|
|
|
|
common.skipIfInspectorDisabled();
|
|
|
|
async function getDebuggedPid(instance, waitForLog = true) {
|
|
const session = await instance.connectInspectorSession();
|
|
await session.send({ method: 'Runtime.enable' });
|
|
if (waitForLog) {
|
|
await session.waitForConsoleOutput('log', 'safe to debug now');
|
|
}
|
|
const { value: innerPid } = (await session.send({
|
|
'method': 'Runtime.evaluate', 'params': { 'expression': 'process.pid' },
|
|
})).result;
|
|
session.disconnect();
|
|
return innerPid;
|
|
}
|
|
|
|
// Triggers a single restart and resolves when the restarted child prints "safe to debug now".
|
|
function restartAndWaitForReady(file, instance) {
|
|
const ready = new Promise((resolve) => {
|
|
instance.on('stdout', (data) => {
|
|
if (data?.includes('safe to debug now')) {
|
|
resolve();
|
|
}
|
|
});
|
|
});
|
|
writeFileSync(file, readFileSync(file));
|
|
return ready;
|
|
}
|
|
|
|
|
|
describe('watch mode - inspect', () => {
|
|
it('should start debugger on inner process', async () => {
|
|
const file = fixtures.path('watch-mode/inspect.js');
|
|
const instance = new NodeInstance(['--inspect=0', '--watch'], undefined, file);
|
|
let stderr = '';
|
|
const stdout = [];
|
|
instance.on('stderr', (data) => { stderr += data; });
|
|
instance.on('stdout', (data) => { stdout.push(data); });
|
|
|
|
const pids = [instance.pid];
|
|
pids.push(await getDebuggedPid(instance));
|
|
instance.resetPort();
|
|
await restartAndWaitForReady(file, instance);
|
|
pids.push(await getDebuggedPid(instance));
|
|
|
|
await instance.kill();
|
|
|
|
// There should be a process per restart and one per parent process.
|
|
// Message about Debugger should appear once per restart.
|
|
// On some systems restart can happen multiple times.
|
|
const restarts = stdout.filter((line) => line === 'safe to debug now').length;
|
|
assert.ok(stderr.match(/Debugger listening on ws:\/\//g).length >= restarts);
|
|
assert.ok(new Set(pids).size >= restarts + 1);
|
|
});
|
|
|
|
it('should prevent attaching debugger with SIGUSR1 to outer process', { skip: common.isWindows }, async () => {
|
|
const file = fixtures.path('watch-mode/inspect_with_signal.js');
|
|
const instance = new NodeInstance(['--inspect-port=0', '--watch'], undefined, file);
|
|
let stderr = '';
|
|
instance.on('stderr', (data) => { stderr += data; });
|
|
|
|
const loggedPid = await new Promise((resolve) => {
|
|
instance.on('stdout', (data) => {
|
|
const matches = data.match(/pid is (\d+)/);
|
|
if (matches) resolve(Number(matches[1]));
|
|
});
|
|
});
|
|
|
|
|
|
process.kill(instance.pid, 'SIGUSR1');
|
|
process.kill(loggedPid, 'SIGUSR1');
|
|
const debuggedPid = await getDebuggedPid(instance, false);
|
|
|
|
await instance.kill();
|
|
|
|
// Message about Debugger should only appear once in inner process.
|
|
assert.strictEqual(stderr.match(/Debugger listening on ws:\/\//g).length, 1);
|
|
assert.strictEqual(loggedPid, debuggedPid);
|
|
});
|
|
});
|