Skip to content

stream: fix Utf8Stream flush handling - #66473

Open
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/utf8stream-flush-fix
Open

jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/utf8stream-flush-fix

Conversation

@jasnell

@jasnell jasnell commented Oct 3, 2026

Copy link
Copy Markdown
Member

Fix several issues with Utf8Stream flushing:

  • flush() now writes buffered data regardless of minLength and invokes the callback only after pending writes complete, including when minLength is zero and a write is in flight.
  • Multiple concurrent flush() calls are tracked correctly and end() waits for pending flushes before closing.
  • flushSync() throws ERR_INVALID_STATE if called while an asynchronous write is in progress instead of corrupting output.
  • Periodic flushes no longer stack up while a flush is pending.
  • fsync is skipped for stdout/stderr file descriptors.
  • reopen(), end() and destroy() behave correctly when the stream is destroyed while still opening.

Separated out from #65840

Fix several issues with `Utf8Stream` flushing:

* `flush()` now writes buffered data regardless of `minLength`
  and invokes the callback only after pending writes complete,
  including when `minLength` is zero and a write is in flight.
* Multiple concurrent `flush()` calls are tracked correctly and
  `end()` waits for pending flushes before closing.
* `flushSync()` throws `ERR_INVALID_STATE` if called while an
  asynchronous write is in progress instead of corrupting output.
* Periodic flushes no longer stack up while a flush is pending.
* `fsync` is skipped for stdout/stderr file descriptors.
* `reopen()`, `end()` and `destroy()` behave correctly when the
  stream is destroyed while still opening.

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
@jasnell
jasnell requested review from mcollina and ronag October 3, 2026 01:48
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to Node.js streams. labels Oct 3, 2026
@codecov

codecov Bot commented Oct 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.69231% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.42%. Comparing base (7fab656) to head (8034879).

Files with missing lines Patch % Lines
lib/internal/streams/fast-utf8-stream.js 87.69% 16 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66473   +/-   ##
=======================================
  Coverage   90.42%   90.42%           
=======================================
  Files         791      791           
  Lines      275569   275662   +93     
  Branches    52842    52888   +46     
=======================================
+ Hits       249185   249273   +88     
+ Misses      16802    16800    -2     
- Partials     9582     9589    +7     
Files with missing lines Coverage Δ
lib/internal/streams/fast-utf8-stream.js 84.14% <87.69%> (+2.77%) ⬆️

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

needs-ci PRs that need a full CI run. stream Issues and PRs related to Node.js streams.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants