vfs: read RealFSProvider files from open fd by trivikr · Pull Request #64104 · nodejs/node · GitHub
Skip to content

vfs: read RealFSProvider files from open fd#64104

Open
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:vfs-readfile-fd-post-rename
Open

vfs: read RealFSProvider files from open fd#64104
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:vfs-readfile-fd-post-rename

Conversation

@trivikr

@trivikr trivikr commented Jun 24, 2026

Copy link
Copy Markdown
Member

Fixes: #64103

RealFileHandle.readFileSync() and readFile() previously reopened the
original real path. If the backing file was renamed after the VFS fd was opened,
fs.readFileSync(fd, ...) failed with ENOENT.

This changes RealFileHandle to read through the already-open real fd using
positioned reads. That matches native filesystem behavior for renamed open files
and preserves the handle's current offset.

The async readFile() path continues to use async stat() and read()
operations.


Assisted-by: openai:gpt-5.5

Read RealFileHandle contents through the open file descriptor instead
of reopening the original real path. This keeps already-open VFS file
descriptors usable after the backing file is renamed.

Use positioned reads so readFileSync() and readFile() preserve the
handle's current offset.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-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 Jun 24, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 24, 2026
@trivikr trivikr requested a review from mcollina June 25, 2026 05:23
Comment thread lib/internal/vfs/providers/real.js Outdated

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.

I don't understand this default size. It seems this is a bug waiting to happen.

If fstatSync does not return the size, this will be initialized by default at 8KB. Can that happen? Why 8KB?

@trivikr trivikr Jun 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

8192 is the default in fs

node/lib/fs.js

Lines 582 to 585 in 4f177f1

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It does introduce a bug where zero-size files were concatenated at 8192 bytes.
It was fixed in 2cef149 (this PR) with a regression test.

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. request-ci Add this label to start a Jenkins CI on a PR. 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 readFileSync(fd) fails after backing file is renamed

3 participants