Skip to content

vfs: keep open memory handles in sync with file - #66356

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:vfs-stale-content-open-handles
Open

trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:vfs-stale-content-open-handles

Conversation

@trivikr

@trivikr trivikr commented Sep 27, 2026

Copy link
Copy Markdown
Member

Fixes: #66355

A MemoryFileHandle snapshotted the file's buffer at open and never checked the entry again. After the file changed through the provider or another handle, an open handle served stale reads and fstat sizes, and its writes went into the old copy, undoing other handles' changes.

Record the entry content each handle last saw or wrote, and adopt the entry's content whenever another handle has replaced it, so an open descriptor is a live view of the file as on a real filesystem.


Assisted-by: claude:opus-5.5

@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 27, 2026
@codecov

codecov Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.50%. Comparing base (06550c7) to head (6dcd3df).
⚠️ Report is 57 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66356      +/-   ##
==========================================
+ Coverage   90.49%   90.50%   +0.01%     
==========================================
  Files         797      797              
  Lines      277811   277833      +22     
  Branches    53366    53367       +1     
==========================================
+ Hits       251407   251455      +48     
+ Misses      16795    16763      -32     
- Partials     9609     9615       +6     
Files with missing lines Coverage Δ
lib/internal/vfs/file_handle.js 99.50% <100.00%> (+0.01%) ⬆️

... and 26 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.

@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

This comment was marked as outdated.

@trivikr
trivikr marked this pull request as draft September 29, 2026 16:58
@pipobscure

Copy link
Copy Markdown
Contributor

#66318 is merged, so this can come outof draft

@pipobscure pipobscure left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@trivikr
trivikr force-pushed the vfs-stale-content-open-handles branch 2 times, most recently from c37b211 to 8dae54c Compare October 18, 2026 23:20
@trivikr
trivikr marked this pull request as ready for review October 18, 2026 23:20
@trivikr
trivikr requested a review from mcollina October 19, 2026 01:15
A MemoryFileHandle snapshotted the file's buffer at open and never
checked the entry again. After the file changed through the provider
or another handle, an open handle served stale reads and fstat sizes,
and its writes went into the old copy, undoing other handles' changes.

Record the entry content each handle last saw or wrote, and adopt the
entry's content whenever another handle has replaced it, so an open
descriptor is a live view of the file as on a real filesystem.

Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
Assisted-by: claude:opus-5.5
@trivikr
trivikr force-pushed the vfs-stale-content-open-handles branch from 8dae54c to 6dcd3df Compare October 19, 2026 02:21
@trivikr

trivikr commented Oct 20, 2026

Copy link
Copy Markdown
Member Author

cc @mcollina Can you take a look?

@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

@trivikr trivikr added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Oct 22, 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 Oct 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 22, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 22, 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

author ready PRs with CI started, the required approvals, and no outstanding review comments. 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: memory provider serves stale content and size to already-open handles

4 participants