-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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 pipe deadlock when starting with needDrain #36563
Conversation
Missing test |
I've updated your test slightly to check for the deadlock: https://gist.github.com/bea83eb0dcbd0aea3b2689c2cd91bb38 |
62844f2
to
0996479
Compare
Seems like we may need to use common.mustCallAtLeast(1) instead of common.mustCall() because the latter requires it to be called exactly once, and it looks like the build system calls pause twice. |
@nodejs/streams |
This was also a separate bug. |
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
It's because pause is called when it's finishing (in unpipe) as well as in the pipe call - so your latest test code looks good to me. |
Landed in ab895bd |
Related Issues
Fixes: #36544
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes