Skip to content

Commit cb8eebf

Browse files
authored
watch: cancel pending restart on shutdown
Cancel any pending FilesWatcher debounce timer when watch mode clears its watchers. This prevents a queued change event from restarting the watched process after shutdown has started. Keep the watched child exit handling attached for the lifetime of the child so overlapping restart and shutdown paths do not remove each other's exit listeners. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63383 Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent e8cc66e commit cb8eebf

4 files changed

Lines changed: 106 additions & 7 deletions

File tree

lib/internal/main/watch_mode.js

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,13 @@ ArrayPrototypeForEach(kWatchedPaths, (p) => watcher.watchPath(p));
8383

8484
let graceTimer;
8585
let child;
86+
let childExitPromise;
8687
let exited;
88+
let stopping;
8789

8890
function start() {
8991
exited = false;
92+
stopping = false;
9093
const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : 'inherit';
9194
child = spawn(process.execPath, argsWithoutWatchOptions, {
9295
stdio,
@@ -103,29 +106,36 @@ function start() {
103106
ArrayPrototypeForEach(kOptionalEnvFiles,
104107
(file) => watcher.filterFile(resolve(file), undefined, { allowMissing: true }));
105108
}
106-
child.once('exit', (code) => {
109+
childExitPromise = once(child, 'exit').then(({ 0: code }) => {
107110
exited = true;
111+
if (stopping) {
112+
return code;
113+
}
108114
const waitingForChanges = 'Waiting for file changes before restarting...';
109115
if (code === 0) {
110116
process.stdout.write(`${blue}Completed running ${kCommandStr}. ${waitingForChanges}${white}\n`);
111117
} else {
112118
process.stdout.write(`${red}Failed running ${kCommandStr}. ${waitingForChanges}${white}\n`);
113119
}
120+
return code;
114121
});
115122
return child;
116123
}
117124

118125
async function killAndWait(signal = kKillSignal, force = false) {
119-
child?.removeAllListeners();
120-
if (!child) {
126+
const processToKill = child;
127+
const onExit = childExitPromise;
128+
if (!processToKill) {
121129
return;
122130
}
123-
if ((child.killed || exited) && !force) {
131+
if ((processToKill.killed || exited) && !force) {
124132
return;
125133
}
126-
const onExit = once(child, 'exit');
127-
child.kill(signal);
128-
const { 0: exitCode } = await onExit;
134+
stopping = true;
135+
if (!exited && processToKill.exitCode === null && processToKill.signalCode === null) {
136+
processToKill.kill(signal);
137+
}
138+
const exitCode = await onExit;
129139
return exitCode;
130140
}
131141

lib/internal/watch_mode/files_watcher.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@ class FilesWatcher extends EventEmitter {
204204
this.#filteredFiles.clear();
205205
}
206206
clear() {
207+
clearTimeout(this.#debounceTimer);
208+
this.#debounceTimer = null;
209+
this.#debounceOwners.clear();
207210
this.#watchers.forEach(this.#unwatch);
208211
this.#watchers.clear();
209212
this.#filteredFiles.clear();
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Flags: --expose-internals
2+
import * as common from '../common/index.mjs';
3+
import tmpdir from '../common/tmpdir.js';
4+
import assert from 'node:assert';
5+
import { writeFileSync } from 'node:fs';
6+
import { createRequire } from 'node:module';
7+
8+
if (common.isIBMi)
9+
common.skip('IBMi does not support `fs.watch()`');
10+
11+
const require = createRequire(import.meta.url);
12+
const timers = require('node:timers');
13+
const originalSetTimeout = timers.setTimeout;
14+
const originalClearTimeout = timers.clearTimeout;
15+
const { promise, resolve } = Promise.withResolvers();
16+
const debounce = 1000;
17+
let debounceTimer;
18+
let debounceTimerCallback;
19+
let debounceTimerCleared = false;
20+
21+
timers.setTimeout = function(fn, delay, ...args) {
22+
// Only intercept the FilesWatcher debounce timer configured below.
23+
if (delay === debounce) {
24+
const timer = {
25+
__proto__: null,
26+
ref() { return this; },
27+
unref() { return this; },
28+
};
29+
debounceTimer = timer;
30+
debounceTimerCallback = () => {
31+
if (!debounceTimerCleared) {
32+
fn(...args);
33+
}
34+
};
35+
resolve();
36+
return timer;
37+
}
38+
return originalSetTimeout(fn, delay, ...args);
39+
};
40+
41+
timers.clearTimeout = function(timer) {
42+
if (timer === debounceTimer) {
43+
debounceTimerCleared = true;
44+
}
45+
return originalClearTimeout(timer);
46+
};
47+
48+
try {
49+
const { FilesWatcher } = require('internal/watch_mode/files_watcher');
50+
51+
tmpdir.refresh();
52+
const file = tmpdir.resolve('watcher-clear.js');
53+
writeFileSync(file, '0');
54+
55+
const watcher = new FilesWatcher({ debounce, mode: 'all' });
56+
watcher.on('changed', common.mustNotCall());
57+
watcher.watchPath(file, false);
58+
59+
const interval = setInterval(() => writeFileSync(file, `${Date.now()}`), 50);
60+
await promise;
61+
clearInterval(interval);
62+
63+
watcher.clear();
64+
assert.strictEqual(debounceTimerCleared, true);
65+
debounceTimerCallback();
66+
} finally {
67+
timers.setTimeout = originalSetTimeout;
68+
timers.clearTimeout = originalClearTimeout;
69+
}

test/sequential/test-watch-mode.mjs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,23 @@ async function failWriteSucceed({ file, watchedFile }) {
171171
tmpdir.refresh();
172172

173173
describe('watch mode', { concurrency: !process.env.TEST_PARALLEL, timeout: 60_000 }, () => {
174+
it('should exit when terminated after the watched process has completed', async () => {
175+
const file = createTmpFile();
176+
const child = spawn(execPath, ['--watch', '--no-warnings', file], {
177+
encoding: 'utf8',
178+
stdio: 'pipe',
179+
});
180+
181+
for await (const line of createInterface({ input: child.stdout })) {
182+
if (line.includes('Completed running')) {
183+
break;
184+
}
185+
}
186+
187+
child.kill();
188+
await once(child, 'exit');
189+
});
190+
174191
it('should watch changes to a file', async () => {
175192
const file = createTmpFile();
176193
const { stderr, stdout } = await runWriteSucceed({ file, watchedFile: file, watchFlag: '--watch=true', options: {

0 commit comments

Comments
 (0)