Skip to content

stream: settle pending stream iter reads on return - #63521

Closed
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:stream-iter-pending-next-hang
Closed

stream: settle pending stream iter reads on return#63521
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:stream-iter-pending-next-hang

Conversation

@trivikr

Copy link
Copy Markdown
Member

This updates stream/iter cleanup so pending next() calls are settled when
a consumer calls return().

Previously, push() and broadcast() could leave a pending next() promise
unresolved if return() was called before any chunk was written. The cleanup
paths now resolve those pending reads with { done: true, value: undefined }.

Regression coverage was added for both push() and broadcast().

Fixes: #63519


Assisted-by: openai:gpt-5.5

@nodejs-github-bot

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

codecovBot commented May 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (8257091) to head (f4a87a4).

Files with missing linesPatch %Lines
lib/internal/streams/iter/push.js28.57%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #63521 +/- ##
==========================================
- Coverage 90.32% 90.31% -0.01% 
==========================================
Files 730 730 Lines 234671 234679 +8 Branches 43946 43950 +4 ==========================================
- Hits 211965 211954 -11 - Misses 14423 14436 +13 - Partials 8283 8289 +6 
Files with missing linesCoverage Δ
lib/internal/streams/iter/broadcast.js85.25% <100.00%> (+0.01%)⬆️
lib/internal/streams/iter/push.js91.00% <28.57%> (-0.58%)⬇️

... and 29 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
Comment threadlib/internal/streams/iter/push.js Outdated
@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

Copy link
Copy Markdown
Collaborator

@trivikrtrivikr added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 24, 2026
@trivikr

This comment was marked as outdated.

@trivikr
trivikr marked this pull request as draft May 24, 2026 04:19
@trivikr
trivikr marked this pull request as ready for review May 26, 2026 17:22
@trivikrtrivikr added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 27, 2026
trivikrand others added 2 commits May 27, 2026 08:39
Resolve pending next() calls when stream/iter push and broadcast
consumers are returned, so the promises do not remain pending after
iterator cleanup.
Fixes: nodejs#63519
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
Co-authored-by: James M Snell <jasnell@gmail.com>
@trivikr
trivikrforce-pushed the stream-iter-pending-next-hang branch from 4c2a5f1 to f4a87a4CompareMay 27, 2026 16:02
@trivikr
trivikr marked this pull request as draft May 27, 2026 17:19
@trivikr

Copy link
Copy Markdown
MemberAuthor

The bug in push was fixed in #63555, when fixing for iterator.throw()

I posted a new broadcast only specific PR at #63603

@trivikrtrivikr closed this May 28, 2026
@trivikr
trivikr deleted the stream-iter-pending-next-hang branch May 28, 2026 00:35
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

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.request-ciAdd this label to start a Jenkins CI on a PR.streamIssues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream/iter: pending next() hangs after iterator return()

3 participants

@trivikr@nodejs-github-bot@jasnell