-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
stream: fix duplexify premature destroy #45133
Conversation
The duplexified Duplex should be autoDestroyed instead of prematurely destroyed when the readable and writable sides have finished without error. Fixes: nodejs#44925
Review requested:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In node v16 has the same code here. But why v16 don't have this issue?
@mcollina what is our procedure to include these kind of PR's in readable-stream? |
Once it ship in a version of Node (ideally the 18 series) I can open a PR update. |
Commit Queue failed- Loading data for nodejs/node/pull/45133 ✔ Done loading data for nodejs/node/pull/45133 ----------------------------------- PR info ------------------------------------ Title stream: fix duplexify premature destroy (#45133) Author Robert Nagy (@ronag) Branch ronag:fix-44925 -> nodejs:main Labels stream, author ready Commits 2 - stream: fix duplexify premature destroy - fixup Committers 1 - Robert Nagy PR-URL: https://github.com/nodejs/node/pull/45133 Fixes: https://github.com/nodejs/node/issues/44925 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Juan José Arboleda ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/45133 Fixes: https://github.com/nodejs/node/issues/44925 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Juan José Arboleda -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last review: ⚠ - fixup ℹ This PR was created on Sun, 23 Oct 2022 13:51:44 GMT ✔ Approvals: 3 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/45133#pullrequestreview-1152327819 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/45133#pullrequestreview-1152332503 ✔ - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/45133#pullrequestreview-1153558445 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2022-10-25T14:51:07Z: https://ci.nodejs.org/job/node-test-pull-request/47472/ - Querying data for job/node-test-pull-request/47472/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/3336681891 |
The duplexified Duplex should be autoDestroyed instead of prematurely destroyed when the readable and writable sides have finished without error. Fixes: #44925 PR-URL: #45133 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Landed in b6eba6b |
The duplexified Duplex should be autoDestroyed instead of prematurely destroyed when the readable and writable sides have finished without error. Fixes: #44925 PR-URL: #45133 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
The duplexified Duplex should be autoDestroyed instead of prematurely destroyed when the readable and writable sides have finished without error. Fixes: #44925 PR-URL: #45133 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
The duplexified Duplex should be autoDestroyed instead of prematurely destroyed when the readable and writable sides have finished without error. Fixes: #44925 PR-URL: #45133 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
The duplexified Duplex should be autoDestroyed instead of prematurely destroyed when the readable and writable sides have finished without error. Fixes: #44925 PR-URL: #45133 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
The duplexified Duplex should be autoDestroyed instead of prematurely destroyed when the readable and writable sides have finished without error. Fixes: #44925 PR-URL: #45133 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
The duplexified Duplex should be autoDestroyed instead of prematurely destroyed when the readable and writable sides have finished without error.
Fixes: #44925