Skip to content

stream: destroy Duplex.from async function on early return - #65963

Open
ac-mmi wants to merge 1 commit into
nodejs:mainfrom
ac-mmi:fix-duplex-from-async-function
Open

stream: destroy Duplex.from async function on early return#65963
ac-mmi wants to merge 1 commit into
nodejs:mainfrom
ac-mmi:fix-duplex-from-async-function

Conversation

@ac-mmi

@ac-mmi ac-mmi commented Sep 10, 2026

Copy link
Copy Markdown

When Duplex.from(asyncFn) is given an AsyncFunction that returns without consuming its async-iterable input, the duplex can remain unfinished, causing pipeline() to hang and preventing the upstream Readable from being destroyed.

Track whether writable final() has started. If the AsyncFunction resolves successfully before final(), destroy the duplex so that pipeline cleanup can propagate upstream.

The change is scoped to the AsyncFunction path and does not modify the shared fromAsyncGen() / async-generator transform behavior.

Fixes: #55077

When an AsyncFunction passed to Duplex.from() resolves without
consuming its input, tear down the duplex so pipeline() can finish
and destroy the upstream readable.

Fixes: nodejs#55077
Assisted-by: Cursor
Signed-off-by: Aman Chadha <79802170+ac-mmi@users.noreply.github.com>
@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 Sep 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.16%. Comparing base (b805fb5) to head (ebf0fde).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65963      +/-   ##
==========================================
- Coverage   90.17%   90.16%   -0.02%     
==========================================
  Files         771      771              
  Lines      265489   265498       +9     
  Branches    50471    50483      +12     
==========================================
- Hits       239405   239377      -28     
- Misses      17042    17051       +9     
- Partials     9042     9070      +28     
Files with missing lines Coverage Δ
lib/internal/streams/duplexify.js 96.98% <100.00%> (+0.06%) ⬆️

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

Stream not destroyed when piped in Duplex.from() writable

2 participants