Skip to content

watch: cancel pending restart on shutdown#63383

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:watch-hang-completed-child-terminate
Open

watch: cancel pending restart on shutdown#63383
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:watch-hang-completed-child-terminate

Conversation

@trivikr
Copy link
Copy Markdown
Member

@trivikr trivikr commented May 17, 2026 •
edited
Loading

Fixes a watch mode shutdown race where a pending debounced file change could still emit after FilesWatcher.clear() closed the active watchers. That allowed watch mode to restart the child process after shutdown had already started, which could leave out/Release/node --watch --inspect processes behind and fail CI’s leftover-process check.

This cancels the pending debounce timer during watcher cleanup and makes watched child exit handling stable across overlapping restart/shutdown paths.

Noticed while examining flaky tests in ubuntu2404_sharedlibs_shared_x64/56356

13:35:08 All tests passed.
13:35:08 ps awwx | grep Release/node | grep -v grep | cat
13:35:08 2405720 ?        Sl     0:00 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/node --inspect=0 --watch /home/iojs/build/workspace/node-test-commit-linux-containered/test/fixtures/watch-mode/inspect.js
13:35:08 2405726 ?        Sl     0:00 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/node --inspect=0 /home/iojs/build/workspace/node-test-commit-linux-containered/test/fixtures/watch-mode/inspect.js
13:35:08 make[1]: *** [Makefile:648: test-ci] Error 1
13:35:08 make: *** [Makefile:674: run-ci] Error 2
13:35:08 Build step 'Conditional steps (multiple)' marked build as failure

Testing

The updated unit tests were run before/after

Before

$ python3 tools/test.py -J --mode=release \
  parallel/test-watch-mode-files_watcher-clear \
  sequential/test-watch-mode-inspect \
  parallel/test-watch-mode-files_watcher
=== release test-watch-mode-files_watcher-clear ===
Path: parallel/test-watch-mode-files_watcher-clear
node:assert:173
  throw err;
  ^

AssertionError [ERR_ASSERTION]: function should not have been called at file:///Users/trivikram/workspace/node/test/parallel/test-watch-mode-files_watcher-clear.mjs:45
called with arguments: { owners: SafeSet(0) {}, eventType: 'change' }
    at FilesWatcher.mustNotCall (/Users/trivikram/workspace/node/test/common/index.js:567:12)
    at FilesWatcher.emit (node:events:509:20)
    at Timeout._onTimeout (node:internal/watch_mode/files_watcher:104:12)
    at listOnTimeout (node:internal/timers:605:17)
    at process.processTimers (node:internal/timers:541:7) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: undefined,
  expected: undefined,
  operator: 'fail',
  diff: 'simple'
}

Node.js v27.0.0-pre
Command: out/Release/node --expose-internals /Users/trivikram/workspace/node/test/parallel/test-watch-mode-files_watcher-clear.mjs


[00:03|% 100|+   2|-   1]: Done

Failed tests:
out/Release/node --expose-internals /Users/trivikram/workspace/node/test/parallel/test-watch-mode-files_watcher-clear.mjs

After

$ python3 tools/test.py -J --mode=release \
  parallel/test-watch-mode-files_watcher-clear \
  sequential/test-watch-mode-inspect \
  parallel/test-watch-mode-files_watcher
[00:03|% 100|+   3|-   0]: Done

All tests passed.

Assisted-by: openai:gpt-5.5

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label May 17, 2026
@trivikr
watch: cancel pending restart on shutdown
7f3594d 
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
@trivikr trivikr force-pushed the watch-hang-completed-child-terminate branch from c0a9c2b to 7f3594d Compare May 17, 2026 15:45
@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.05%. Comparing base (a9b0a83) to head (7f3594d).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #63383      +/-   ##
==========================================
- Coverage   90.06%   90.05%   -0.02%     
==========================================
  Files         714      714              
  Lines      225629   225632       +3     
  Branches    42702    42673      -29     
==========================================
- Hits       203211   203191      -20     
+ Misses      14232    14230       -2     
- Partials     8186     8211      +25
Files with missing lines Coverage Δ
lib/internal/watch_mode/files_watcher.js 91.28% <100.00%> (+0.58%) ⬆️

... and 52 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trivikr trivikr added watch-mode Issues and PRs related to watch mode request-ci Add this label to start a Jenkins CI on a PR. labels May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. watch-mode Issues and PRs related to watch mode

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@trivikr @nodejs-github-bot