stream: destroy duplex on early return in Duplex.from(asyncFn)#62953
Open
maruthang wants to merge 1 commit intonodejs:mainfrom
Open
stream: destroy duplex on early return in Duplex.from(asyncFn)#62953maruthang wants to merge 1 commit intonodejs:mainfrom
maruthang wants to merge 1 commit intonodejs:mainfrom
Conversation
When `Duplex.from(asyncFn)` is given an `async function (gen)` that returns without consuming all of `gen`, the duplex never reached `final`, so the pipeline kept pumping writes that never completed and the upstream Readable was never destroyed. Track whether `final()` has been called via a `finalized` flag. When the user's promise resolves and `final` has not been called, call `destroyer(d)` on the duplex so the existing `eos` handler propagates destruction upstream. The async-iterable branch (`isIterable(value)`) and the rejection path are unmodified, so the regression that caused PR nodejs#55096 to be reverted by PR nodejs#56278 is preserved. Fixes: nodejs#55077 Signed-off-by: Maruthan G <[email protected]>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
needs-ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #62953 +/- ##
==========================================
- Coverage 89.66% 89.64% -0.03%
==========================================
Files 706 706
Lines 219391 219399 +8
Branches 42068 42067 -1
==========================================
- Hits 196712 196672 -40
- Misses 14578 14631 +53
+ Partials 8101 8096 -5
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
Duplex.from(asyncFn)is given anasync function (gen)that returns without consuming all of
gen, the duplex neverreached
final, so the pipeline kept pumping writes that nevercompleted and the upstream Readable was never destroyed.
Track whether
final()has been called via afinalizedflag.When the user's promise resolves and
finalhas not beencalled, call
destroyer(d)on the duplex so the existingeoshandler propagates destruction upstream.
The async-iterable branch (
isIterable(value)) and therejection path are unmodified, so the regression that caused
PR #55096 to be reverted by PR #56278 is preserved.
Fixes: #55077
Note: I was unable to run the test suite locally (no built
out/Release/nodeon this Windows host). The touched JS lints clean and passesnode --check. The PR #56278 regression scenario was carefully preserved by leaving theisIterable(value)branch untouched. Looking forward to CI verification — please flag any concern about reintroducing PR #56278's regression.