Skip to content

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

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
ac-mmi:fix-duplex-from-async-function
Sep 18, 2026
Merged

nodejs-github-bot merged 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
Contributor

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

@meixg meixg added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 11, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 11, 2026
@nodejs-github-bot

This comment has been minimized.

@panva panva added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 11, 2026
@jasnell
jasnell requested review from mcollina and ronag September 11, 2026 18:26
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ac-mmi

ac-mmi commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@jasnell I noticed the Jenkins CI run has 4 failing checks, but I don't have access to the Jenkins logs. Could you please take a look and let me know whether the failures are related to this change or infrastructure/flakiness? Thanks!

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@inoway46 inoway46 added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 18, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 0bf9e9f into nodejs:main Sep 18, 2026
87 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 0bf9e9f

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 18, 2026
aduh95 pushed a commit that referenced this pull request Sep 18, 2026
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: #55077
Assisted-by: Cursor
Signed-off-by: Aman Chadha <79802170+ac-mmi@users.noreply.github.com>
PR-URL: #65963
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. 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

5 participants