stream: destroy duplex on early return in Duplex.from(asyncFn) - #62953
stream: destroy duplex on early return in Duplex.from(asyncFn)#62953maruthang wants to merge 2 commits into
Conversation
Sorry, something went wrong.
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 <maruthang4@gmail.com>
|
Review requested:
|
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ 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
... and 32 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
|
The single red check on the previous run was the well-known parallel/test-watch-mode-files_watcher macOS flake (TIMEOUT after ..................................... failed 1 out of 10 retries): === release test-watch-mode-files_watcher === Path: parallel/test-watch-mode-files_watcher ##[error]Command: out/Release/node --expose-internals --test-reporter=./test/common/test-error-reporter.js --test-reporter-destination=stdout test/parallel/test-watch-mode-files_watcher.mjs --- TIMEOUT --- === 1 tests failed The change here is lib/internal/streams/duplexify.js-only and cannot affect the watch-mode/files-watcher path. Pushed an empty commit to retrigger the macOS slice. |
Sorry, something went wrong.
|
This pull request has been marked as stale due to 90 days of inactivity. |
Sorry, something went wrong.
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 #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/node on this Windows host). The touched JS lints clean and passes node --check. The PR #56278 regression scenario was carefully preserved by leaving the isIterable(value) branch untouched. Looking forward to CI verification — please flag any concern about reintroducing PR #56278's regression.