Skip to content

fs: use correct position when retrying writev - #66203

Open
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:fs-writev-retry-position
Open

christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:fs-writev-retry-position

Conversation

@christianaurichzm

Copy link
Copy Markdown
Contributor

WriteStream retries a partial writev() at the wrong position when start is set.

writevAll() tracks the next position in pos, but passes this.pos to writev(). _writev() advances this.pos by the full batch size before the callback runs, so a short write can retry from the end of the batch instead of after the bytes that were written.

For a 12-byte batch starting at 0, with the first call writing 3 bytes:

writev at 0
writev at 12

The second call should be at 3. The current result is:

"AAA\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000ABBBBCCCC"

EAGAIN has the same problem: the retry uses the advanced this.pos even though no bytes were written.

Pass pos instead, matching writeAll().

The regression test covers partial writes at start: 0 and start: 5, EAGAIN, and the unpositioned case.

For start: 5:

partial write: [5, 17] -> [5, 8]
EAGAIN:        [5, 17] -> [5, 5]

make lint passes. parallel and sequential were also run on Linux x64. Four parallel failures related to --use-openssl-ca also reproduce on the base commit.

Refs: #49211

writevAll() tracks the position for the next attempt in its `pos`
argument, but calls fs.writev() with `this.pos`. WriteStream._writev()
advances `this.pos` by the whole batch right after starting the write,
so a retry from the asynchronous callback resumes at the end of the
batch instead of after the bytes that reached the file. A partial write
leaves a hole and puts the remainder past its place; an EAGAIN retry
moves forward even though nothing was written.

Use the local `pos`, as writeAll() already does.

Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. labels Sep 22, 2026
@codecov

codecov Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.29%. Comparing base (36dd044) to head (f6535a4).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66203   +/-   ##
=======================================
  Coverage   90.29%   90.29%           
=======================================
  Files         790      790           
  Lines      272529   272529           
  Branches    52031    52033    +2     
=======================================
+ Hits       246083   246089    +6     
+ Misses      16909    16897   -12     
- Partials     9537     9543    +6     
Files with missing lines Coverage Δ
lib/internal/fs/streams.js 94.45% <100.00%> (+1.34%) ⬆️

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

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

Labels

fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants