Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable passthrough IPC in watch mode #50890

Merged
merged 9 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 21 additions & 11 deletions lib/internal/main/watch_mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ function start() {
process.stdout.write(`${red}Failed running ${kCommandStr}${white}\n`);
}
});
return child;
}

async function killAndWait(signal = kKillSignal, force = false) {
Expand Down Expand Up @@ -113,34 +114,43 @@ function reportGracefulTermination() {
};
}

async function stop() {
async function stop(child) {
// Without this line, the child process is still able to receive IPC, but is unable to send additional messages
watcher.destroyIPC(child);
watcher.clearFileFilters();
const clearGraceReport = reportGracefulTermination();
await killAndWait();
clearGraceReport();
}

let restarting = false;
async function restart() {
async function restart(child) {
if (restarting) return;
restarting = true;
try {
if (!kPreserveOutput) process.stdout.write(clear);
process.stdout.write(`${green}Restarting ${kCommandStr}${white}\n`);
await stop();
start();
await stop(child);
return start();
} finally {
restarting = false;
}
}

start();
watcher
.on('changed', restart)
.on('error', (error) => {
watcher.off('changed', restart);
triggerUncaughtException(error, true /* fromPromise */);
});
async function init() {
let child = start();
const restartChild = async () => {
child = await restart(child);
};
watcher
.on('changed', restartChild)
.on('error', (error) => {
watcher.off('changed', restartChild);
triggerUncaughtException(error, true /* fromPromise */);
});
}

init();

// Exiting gracefully to avoid stdout/stderr getting written after
// parent process is killed.
Expand Down
29 changes: 29 additions & 0 deletions lib/internal/watch_mode/files_watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
const {
ArrayIsArray,
ArrayPrototypeForEach,
Boolean,
SafeMap,
znewsham marked this conversation as resolved.
Show resolved Hide resolved
SafeSet,
SafeWeakMap,
StringPrototypeStartsWith,
} = primordials;

Expand All @@ -31,6 +33,8 @@ class FilesWatcher extends EventEmitter {
#debounce;
#mode;
#signal;
#passthroughIPC = false;
#ipcHandlers = new SafeWeakMap();

constructor({ debounce = 200, mode = 'filter', signal } = kEmptyObject) {
super({ __proto__: null, captureRejections: true });
Expand All @@ -40,6 +44,7 @@ class FilesWatcher extends EventEmitter {
this.#debounce = debounce;
this.#mode = mode;
this.#signal = signal;
this.#passthroughIPC = Boolean(process.send);

if (signal) {
addAbortListener(signal, () => this.clear());
Expand Down Expand Up @@ -128,7 +133,31 @@ class FilesWatcher extends EventEmitter {
this.#ownerDependencies.set(owner, dependencies);
}
}


#setupIPC(child) {
const handlers = {
__proto__: null,
parentToChild: (message) => child.send(message),
childToParent: (message) => process.send(message),
};
this.#ipcHandlers.set(child, handlers);
process.on('message', handlers.parentToChild);
child.on('message', handlers.childToParent);
}

destroyIPC(child) {
const handlers = this.#ipcHandlers.get(child);
if (this.#passthroughIPC && handlers !== undefined) {
process.off('message', handlers.parentToChild);
child.off('message', handlers.childToParent);
}
}

watchChildProcessModules(child, key = null) {
if (this.#passthroughIPC) {
this.#setupIPC(child);
}
if (this.#mode !== 'filter') {
return;
}
Expand Down
81 changes: 81 additions & 0 deletions test/sequential/test-watch-mode.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { writeFileSync, readFileSync, mkdirSync } from 'node:fs';
import { inspect } from 'node:util';
import { pathToFileURL } from 'node:url';
import { once } from 'node:events';
import { createInterface } from 'node:readline';

if (common.isIBMi)
Expand Down Expand Up @@ -574,4 +575,84 @@
`Completed running ${inspect(file)}`,
]);
});
znewsham marked this conversation as resolved.
Show resolved Hide resolved

it('should pass IPC messages from a spawning parent to the child and back', async () => {
const file = createTmpFile(`console.log('running');
process.on('message', (message) => {
if (message === 'exit') {
process.exit(0);
} else {
console.log('Received:', message);
process.send(message);
}
})`);

const child = spawn(
znewsham marked this conversation as resolved.
Show resolved Hide resolved
execPath,
[
'--watch',
'--no-warnings',
file,
],
{
encoding: 'utf8',
stdio: ['pipe', 'pipe', 'pipe', 'ipc'],
},
);

let stderr = '';
let stdout = '';

child.stdout.on('data', (data) => stdout += data);
child.stderr.on('data', (data) => stderr += data);
async function waitForEcho(msg) {
const receivedPromise = new Promise((resolve) => {
const fn = (message) => {
if (message === msg) {
child.off('message', fn);
resolve();
}
};
child.on('message', fn);
});
child.send(msg);
await receivedPromise;
}

async function waitForText(text) {
const seenPromise = new Promise((resolve) => {
const fn = (data) => {
if (data.toString().includes(text)) {
resolve();
child.stdout.off('data', fn);
}
};
child.stdout.on('data', fn);
});
await seenPromise;
}

await waitForText('running');
await waitForEcho('first message');
const stopRestarts = restart(file);
await waitForText('running');
stopRestarts();
await waitForEcho('second message');
const exitedPromise = once(child, 'exit');
child.send('exit');
await waitForText('Completed');
child.disconnect();
child.kill();
await exitedPromise;
assert.strictEqual(stderr, '');
const lines = stdout.split(/\r?\n/).filter(Boolean);
assert.deepStrictEqual(lines, [

Check failure on line 649 in test/sequential/test-watch-mode.mjs

View workflow job for this annotation

GitHub Actions / test-macOS

--- stdout --- ▶ watch mode ✔ should watch changes to a file (370.692833ms) ::debug::starting to run watch mode ::debug::starting to run should watch changes to a file ::debug::completed running should watch changes to a file ✔ should watch changes to a file - event loop ended (367.248625ms) ::debug::starting to run should watch changes to a file - event loop ended ::debug::completed running should watch changes to a file - event loop ended ✔ should watch changes to a failing file (361.026125ms) ::debug::starting to run should watch changes to a failing file ::debug::completed running should watch changes to a failing file ✔ should watch changes to a file with watch-path (364.730208ms) ::debug::starting to run should watch changes to a file with watch-path ::debug::completed running should watch changes to a file with watch-path ✔ should watch when running an non-existing file - when specified under --watch-path (372.064083ms) ::debug::starting to run should watch when running an non-existing file - when specified under --watch-path ::debug::completed running should watch when running an non-existing file - when specified under --watch-path ✔ should watch when running an non-existing file - when specified under --watch-path with equals (364.170708ms) ::debug::starting to run should watch when running an non-existing file - when specified under --watch-path with equals ::debug::completed running should watch when running an non-existing file - when specified under --watch-path with equals ✔ should watch changes to a file - event loop blocked (370.459792ms) ::debug::starting to run should watch changes to a file - event loop blocked ::debug::completed running should watch changes to a file - event loop blocked ✔ should watch changes to dependencies - cjs (372.941458ms) ::debug::starting to run should watch changes to dependencies - cjs ::debug::completed running should watch changes to dependencies - cjs ✔ should watch changes to dependencies - esm (366.038542ms) ::debug::starting to run should watch changes to dependencies - esm ::debug::completed running should watch changes to dependencies - esm ✔ should restart multiple times (2873.108167ms) ::debug::starting to run should restart multiple times ::debug::completed running should restart multiple times ✔ should pass arguments to file (367.232375ms) ::debug::starting to run should pass arguments to file ::debug::completed running should pass arguments to file ✔ should load --require modules in the watched process, and not in the orchestrator process (369.964625ms) ::debug::starting to run should load --require modules in the watched process, and not in the orchestrator process ::debug::completed running should load --require modules in the watched process, and not in the orchestrator process ✔ should load --import modules in the watched process, and not in the orchestrator process (382.217042ms) ::debug::starting to run should load --import modules in the watched process, and not in the orchestrator process ::debug::completed running should load --import modules in the watched process, and not in the orchestrator process ✔ should not watch when running an missing file (367.507042ms) ::debug::starting to run should not watch when running an missing file ::debug::completed running should not watch when running an missing file ✔ should not watch when running an missing mjs file (376.777541ms) ::debug::starting to run should not watch when running an missing mjs file ::debug::completed running should not watch when running an missing mjs file ✔ should watch changes to previously missing dependency (347.1295ms) ::debug::starting to run should watch changes to previously missing dependency ::debug::completed running should watch changes to previously missing dependency ✔ should watch changes to previously missing ESM dependency (359.737916ms) ::debug::starting to run should watch changes to previously missing ESM dependency ::debug::completed running should watch changes to previously missing ESM dependency ✔ should clear output between runs

Check failure on line 649 in test/sequential/test-watch-mode.mjs

View workflow job for this annotation

GitHub Actions / test-linux

--- stdout --- ▶ watch mode ✔ should watch changes to a file (282.750659ms) ::debug::starting to run watch mode ::debug::starting to run should watch changes to a file ::debug::completed running should watch changes to a file ✔ should watch changes to a file - event loop ended (282.438108ms) ::debug::starting to run should watch changes to a file - event loop ended ::debug::completed running should watch changes to a file - event loop ended ✔ should watch changes to a failing file (280.042237ms) ﹣ should watch changes to a file with watch-path (0.208361ms) # SKIP ﹣ should watch when running an non-existing file - when specified under --watch-path (0.124012ms) # SKIP ﹣ should watch when running an non-existing file - when specified under --watch-path with equals (0.159259ms) # SKIP ::debug::starting to run should watch changes to a failing file ::debug::completed running should watch changes to a failing file ::debug::starting to run should watch changes to a file with watch-path ::debug::completed running should watch changes to a file with watch-path ::debug::starting to run should watch when running an non-existing file - when specified under --watch-path ::debug::completed running should watch when running an non-existing file - when specified under --watch-path ::debug::starting to run should watch when running an non-existing file - when specified under --watch-path with equals ::debug::completed running should watch when running an non-existing file - when specified under --watch-path with equals ✔ should watch changes to a file - event loop blocked (276.540257ms) ::debug::starting to run should watch changes to a file - event loop blocked ::debug::completed running should watch changes to a file - event loop blocked ✔ should watch changes to dependencies - cjs (279.745048ms) ::debug::starting to run should watch changes to dependencies - cjs ::debug::completed running should watch changes to dependencies - cjs ✔ should watch changes to dependencies - esm (294.118145ms) ::debug::starting to run should watch changes to dependencies - esm ::debug::completed running should watch changes to dependencies - esm ✔ should restart multiple times (2782.008913ms) ::debug::starting to run should restart multiple times ::debug::completed running should restart multiple times ✔ should pass arguments to file (283.916935ms) ::debug::starting to run should pass arguments to file ::debug::completed running should pass arguments to file ✔ should load --require modules in the watched process, and not in the orchestrator process (282.475038ms) ::debug::starting to run should load --require modules in the watched process, and not in the orchestrator process ::debug::completed running should load --require modules in the watched process, and not in the orchestrator process ✔ should load --import modules in the watched process, and not in the orchestrator process (292.818587ms) ﹣ should not watch when running an missing file (0.202118ms) # SKIP ﹣ should not watch when running an missing mjs file (0.112681ms) # SKIP ﹣ should watch changes to previously missing dependency (0.102663ms) # SKIP ::debug::starting to run should load --import modules in the watched process, and not in the orchestrator process ::debug::completed running should load --import modules in the watched process, and not in the orchestrator process ::debug::starting to run should not watch when running an missing file ::debug::completed running should not watch when running an missing file ::debug::starting to run should not watch when running an missing mjs file ::debug::completed running should not watch when running an missing mjs file ::debug::starting to run should watch changes to previously missing dependency ::debug::completed running should watch changes to previously missing dependency ﹣ should watch changes to previously missing ESM dependency (0.134111ms) # SKIP ::debug::starting to run should watch changes to previously missing ESM dependency ::debug::completed running should watch changes to previously missing ESM dependency

Check failure on line 649 in test/sequential/test-watch-mode.mjs

View workflow job for this annotation

GitHub Actions / test-asan

--- stdout --- ▶ watch mode ✔ should watch changes to a file (454.646653ms) ::debug::starting to run watch mode ::debug::starting to run should watch changes to a file ::debug::completed running should watch changes to a file ✔ should watch changes to a file - event loop ended (451.236422ms) ::debug::starting to run should watch changes to a file - event loop ended ::debug::completed running should watch changes to a file - event loop ended ✔ should watch changes to a failing file (445.418423ms) ﹣ should watch changes to a file with watch-path (0.635161ms) # SKIP ﹣ should watch when running an non-existing file - when specified under --watch-path (0.191727ms) # SKIP ﹣ should watch when running an non-existing file - when specified under --watch-path with equals (0.373776ms) # SKIP ::debug::starting to run should watch changes to a failing file ::debug::completed running should watch changes to a failing file ::debug::starting to run should watch changes to a file with watch-path ::debug::completed running should watch changes to a file with watch-path ::debug::starting to run should watch when running an non-existing file - when specified under --watch-path ::debug::completed running should watch when running an non-existing file - when specified under --watch-path ::debug::starting to run should watch when running an non-existing file - when specified under --watch-path with equals ::debug::completed running should watch when running an non-existing file - when specified under --watch-path with equals ✔ should watch changes to a file - event loop blocked (392.061166ms) ::debug::starting to run should watch changes to a file - event loop blocked ::debug::completed running should watch changes to a file - event loop blocked ✔ should watch changes to dependencies - cjs (448.806652ms) ::debug::starting to run should watch changes to dependencies - cjs ::debug::completed running should watch changes to dependencies - cjs ✔ should watch changes to dependencies - esm (508.821015ms) ::debug::starting to run should watch changes to dependencies - esm ::debug::completed running should watch changes to dependencies - esm ✔ should restart multiple times (2946.282977ms) ::debug::starting to run should restart multiple times ::debug::completed running should restart multiple times ✔ should pass arguments to file (454.264898ms) ::debug::starting to run should pass arguments to file ::debug::completed running should pass arguments to file ✔ should load --require modules in the watched process, and not in the orchestrator process (450.980059ms) ::debug::starting to run should load --require modules in the watched process, and not in the orchestrator process ::debug::completed running should load --require modules in the watched process, and not in the orchestrator process ✔ should load --import modules in the watched process, and not in the orchestrator process (498.699595ms) ﹣ should not watch when running an missing file (0.223566ms) # SKIP ﹣ should not watch when running an missing mjs file (0.121796ms) # SKIP ﹣ should watch changes to previously missing dependency (0.101068ms) # SKIP ::debug::starting to run should load --import modules in the watched process, and not in the orchestrator process ::debug::completed running should load --import modules in the watched process, and not in the orchestrator process ::debug::starting to run should not watch when running an missing file ::debug::completed running should not watch when running an missing file ::debug::starting to run should not watch when running an missing mjs file ::debug::completed running should not watch when running an missing mjs file ::debug::starting to run should watch changes to previously missing dependency ::debug::completed running should watch changes to previously missing dependency ﹣ should watch changes to previously missing ESM dependency (0.229777ms) # SKIP ::debug::starting to run should watch changes to previously missing ESM dependency ::debug::completed running should watch changes to previously missing ESM dependency
'running',
'Received: first message',
`Restarting '${inspect(file)}'`,
'running',
'Received: second message',
`Completed running ${inspect(file)}`,
]);
});
});