Skip to content

vfs: forward the watchFile listener through RealFSProvider - #66224

Open
trivikr wants to merge 4 commits into
nodejs:mainfrom
trivikr:vfs-watchfile-listener-ignore
Open

trivikr wants to merge 4 commits into
nodejs:mainfrom
trivikr:vfs-watchfile-listener-ignore

Conversation

@trivikr

@trivikr trivikr commented Sep 23, 2026

Copy link
Copy Markdown
Member

Fixes: #66223

fs.watchFile() on a path served by a real-directory mount dispatches to RealFSProvider.watchFile(), which ignored the supplied listener and registered an empty function instead. The user callback never fired after the file changed, and fs.unwatchFile(path, listener) could not match the anonymous stub, so the stat watcher's poll timer stayed registered and kept the process alive.

Accept the listener and pass it through to fs.watchFile() so the callback fires on change and unwatchFile() removes it by identity.


Assisted-by: opencode

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem. labels Sep 23, 2026
@trivikr
trivikr force-pushed the vfs-watchfile-listener-ignore branch from 6f9a2df to 50f2740 Compare September 23, 2026 04:04
@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.69%. Comparing base (2758c59) to head (0e9056a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66224      +/-   ##
==========================================
- Coverage   92.99%   90.69%   -2.31%     
==========================================
  Files         428      797     +369     
  Lines      195537   278787   +83250     
  Branches    30252    53608   +23356     
==========================================
+ Hits       181845   252846   +71001     
- Misses      13365    16299    +2934     
- Partials      327     9642    +9315     
Files with missing lines Coverage Δ
lib/internal/vfs/providers/real.js 95.68% <100.00%> (ø)

... and 503 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 force-pushed the vfs-watchfile-listener-ignore branch from dc9c13b to 1cf3d97 Compare September 23, 2026 06:12
@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Sep 23, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@trivikr
trivikr force-pushed the vfs-watchfile-listener-ignore branch from 1cf3d97 to 9401849 Compare September 27, 2026 21:20
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr
trivikr requested a review from mcollina September 28, 2026 15:32
@trivikr

trivikr commented Oct 23, 2026

Copy link
Copy Markdown
Member Author

cc @mcollina Can you take a look?

fs.watchFile() on a path served by a real-directory mount dispatches
to RealFSProvider.watchFile(), which ignored the supplied listener and
registered an empty function instead. The user callback never fired
after the file changed, and fs.unwatchFile(path, listener) could not
match the anonymous stub, so the stat watcher's poll timer stayed
registered and kept the process alive.

Accept the listener and pass it through to fs.watchFile() so the
callback fires on change and unwatchFile() removes it by identity.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: opencode
@trivikr
trivikr force-pushed the vfs-watchfile-listener-ignore branch from 9401849 to 0b64419 Compare November 1, 2026 17:57

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 2, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 2, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vfs: RealFSProvider.watchFile ignores the listener and unwatchFile leaks it

3 participants