Skip to content

Commit 28f380d

Browse files
trivikraduh95
authored andcommitted
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 683de5d commit 28f380d

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
@@ -87,10 +87,13 @@ ArrayPrototypeForEach(kWatchedPaths, (p) => watcher.watchPath(p));
8787

8888
let graceTimer;
8989
let child;
90+
let childExitPromise;
9091
let exited;
92+
let stopping;
9193

9294
function start() {
9395
exited = false;
96+
stopping = false;
9497
const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : 'inherit';
9598
child = spawn(process.execPath, argsWithoutWatchOptions, {
9699
stdio,
@@ -107,29 +110,36 @@ function start() {
107110
ArrayPrototypeForEach(kOptionalEnvFiles,
108111
(file) => watcher.filterFile(resolve(file), undefined, { allowMissing: true }));
109112
}
110-
child.once('exit', (code) => {
113+
childExitPromise = once(child, 'exit').then(({ 0: code }) => {
111114
exited = true;
115+
if (stopping) {
116+
return code;
117+
}
112118
const waitingForChanges = 'Waiting for file changes before restarting...';
113119
if (code === 0) {
114120
process.stdout.write(`${blue}Completed running ${kCommandStr}. ${waitingForChanges}${white}\n`);
115121
} else {
116122
process.stdout.write(`${red}Failed running ${kCommandStr}. ${waitingForChanges}${white}\n`);
117123
}
124+
return code;
118125
});
119126
return child;
120127
}
121128

122129
async function killAndWait(signal = kKillSignal, force = false) {
123-
child?.removeAllListeners();
124-
if (!child) {
130+
const processToKill = child;
131+
const onExit = childExitPromise;
132+
if (!processToKill) {
125133
return;
126134
}
127-
if ((child.killed || exited) && !force) {
135+
if ((processToKill.killed || exited) && !force) {
128136
return;
129137
}
130-
const onExit = once(child, 'exit');
131-
child.kill(signal);
132-
const { 0: exitCode } = await onExit;
138+
stopping = true;
139+
if (!exited && processToKill.exitCode === null && processToKill.signalCode === null) {
140+
processToKill.kill(signal);
141+
}
142+
const exitCode = await onExit;
133143
return exitCode;
134144
}
135145

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)