Skip to content

stream: wait for push writer end fallback to drain - #63503

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:stream-iter-push-writer-end-resolve
May 25, 2026
Merged

stream: wait for push writer end fallback to drain#63503
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:stream-iter-push-writer-end-resolve

Conversation

@trivikr

@trivikrtrivikr commented May 23, 2026•
edited
Loading

Copy link
Copy Markdown
Member

Fixes an issue in node:stream/iter where push() writer end() could
resolve before buffered data was consumed when called after endSync()
returned -1.

endSync() already transitions the queue to closing when data remains
buffered. A subsequent async end() now keeps waiting for that closing
state to drain, and reuses any existing pending end promise.

duplex() channel close now only signals EOF synchronously instead of
awaiting that drain, since the peer may not start reading until after
close() resolves.

Fixes: #63502

Assisted-by: openai:gpt-5.5

@nodejs-github-bot

nodejs-github-bot commented May 23, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels May 23, 2026
@trivikr
stream: wait for push writer end fallback to drain
cee2067
When endSync() returns -1 after buffered writes, a follow-up end()
should stay pending until the readable side drains the queued data.
Do not make duplex channel close() wait for that drain, since close()
only needs to signal EOF to the peer. Waiting there can deadlock when
the peer starts reading only after close() resolves.
Fixes: nodejs#63502
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr
trivikrforce-pushed the stream-iter-push-writer-end-resolve branch from 7fa1d26 to cee2067CompareMay 23, 2026 06:50
@codecov

codecovBot commented May 23, 2026•
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.13%. Comparing base (8d3245e) to head (cee2067).
⚠️ Report is 15 commits behind head on main.

Files with missing linesPatch %Lines
lib/internal/streams/iter/push.js62.50%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #63503 +/- ##
==========================================
- Coverage 90.14% 90.13% -0.02% 
==========================================
Files 718 718 Lines 227984 227987 +3 Branches 42835 42841 +6 ==========================================
- Hits 205522 205486 -36 - Misses 14235 14284 +49 + Partials 8227 8217 -10
Files with missing linesCoverage Δ
lib/internal/streams/iter/duplex.js96.35% <100.00%> (-0.11%)⬇️
lib/internal/streams/iter/push.js92.01% <62.50%> (-0.20%)⬇️

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

@trivikrtrivikr added the request-ci Add this label to start a Jenkins CI on a PR. label May 23, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 23, 2026
@nodejs-github-bot

nodejs-github-bot commented May 23, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/73638/

@trivikrtrivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 24, 2026
mcollina
mcollina approved these changes May 24, 2026
View reviewed changes

@mcollinamcollina 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

@trivikrtrivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label May 24, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 25, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 8c495c8 into nodejs:mainMay 25, 2026
97 of 99 checks passed
@nodejs-github-bot

nodejs-github-bot commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Landed in 8c495c8

@trivikr
trivikr deleted the stream-iter-push-writer-end-resolve branch May 27, 2026 07:49
aduh95 pushed a commit that referenced this pull request May 27, 2026
@trivikr@aduh95
stream: wait for push writer end fallback to drain
dc12b73
When endSync() returns -1 after buffered writes, a follow-up end()
should stay pending until the readable side drains the queued data.
Do not make duplex channel close() wait for that drain, since close()
only needs to signal EOF to the peer. Waiting there can deadlock when
the peer starts reading only after close() resolves.
Fixes: #63502
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63503Fixes: #63502
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@github-actionsgithub-actionsBot mentioned this pull request May 30, 2026
Merged
@aduh95aduh95 mentioned this pull request Jul 22, 2026
Closed
aduh95 pushed a commit to aduh95/node that referenced this pull request Jul 30, 2026
@trivikr@aduh95
stream: wait for push writer end fallback to drain
b943441
When endSync() returns -1 after buffered writes, a follow-up end()
should stay pending until the readable side drains the queued data.
Do not make duplex channel close() wait for that drain, since close()
only needs to signal EOF to the peer. Waiting there can deadlock when
the peer starts reading only after close() resolves.
Fixes: nodejs#63502
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: nodejs#63503Fixes: nodejs#63502
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
@trivikr@aduh95
stream: wait for push writer end fallback to drain
45780df
When endSync() returns -1 after buffered writes, a follow-up end()
should stay pending until the readable side drains the queued data.
Do not make duplex channel close() wait for that drain, since close()
only needs to signal EOF to the peer. Waiting there can deadlock when
the peer starts reading only after close() resolves.
Fixes: #63502
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63503
Backport-PR-URL: #64675Fixes: #63502
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@jasnelljasnelljasnell approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.streamIssues and PRs related to the stream subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

stream/iter: push writer.end() resolves before readable drains buffered data

4 participants

@trivikr@nodejs-github-bot@mcollina@jasnell