stream: fix readable being emitted when nothing to do by mafintosh · Pull Request #18372 · nodejs/node (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation6 Commits1 Checks0 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes- tests and/or benchmarks are included
- documentation is changed or added
- commit message follows commit guidelines
Affected core subsystem(s)
stream
This fixes an issue introduced by #17979 where the readable event are sometimes emitted when the stream is neither readable nor has it ended.
CITGM seems similar to master, landing.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
mcollina pushed a commit that referenced this pull request
Fixes a regression introduced by the once-per-microtick 'readable' event emission.
See: #17979 PR-URL: #18372 Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Matteo Collina matteo.collina@gmail.com
MoonBall pushed a commit to MoonBall/node that referenced this pull request
The original test case hides the underlying cause by using
PassThrough. This change adds a test case for the underlying cause.
This makes it clearer and easier to be understood.
Refs: nodejs#18372
mcollina pushed a commit that referenced this pull request
The original test case hides the underlying cause by using
PassThrough. This change adds a test case for the underlying cause.
This makes it clearer and easier to be understood.
Refs: #18372
PR-URL: #18575 Reviewed-By: Matteo Collina matteo.collina@gmail.com
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request
Fixes a regression introduced by the once-per-microtick 'readable' event emission.
See: nodejs#17979 PR-URL: nodejs#18372 Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Matteo Collina matteo.collina@gmail.com
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request
The original test case hides the underlying cause by using
PassThrough. This change adds a test case for the underlying cause.
This makes it clearer and easier to be understood.
Refs: nodejs#18372
PR-URL: nodejs#18575 Reviewed-By: Matteo Collina matteo.collina@gmail.com
lundibundi added a commit to lundibundi/node that referenced this pull request
Avoid trying to emit 'readable' due to the fact that state.length is always >= state.highWaterMark if highWaterMark is 0. Therefore upon .read(0) call (through .on('readable')) stream assumed that it has enough data to emit 'readable' even though state.length === 0 instead of issuing _read(). Which led to the TTY not recognizing that someone is waiting for the input.
Fixes: nodejs#20503 Refs: nodejs#18372
mcollina pushed a commit that referenced this pull request
Avoid trying to emit 'readable' due to the fact that state.length is always >= state.highWaterMark if highWaterMark is 0. Therefore upon .read(0) call (through .on('readable')) stream assumed that it has enough data to emit 'readable' even though state.length === 0 instead of issuing _read(). Which led to the TTY not recognizing that someone is waiting for the input.
PR-URL: #21690 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Ruben Bridgewater ruben@bridgewater.de
targos pushed a commit that referenced this pull request
Avoid trying to emit 'readable' due to the fact that state.length is always >= state.highWaterMark if highWaterMark is 0. Therefore upon .read(0) call (through .on('readable')) stream assumed that it has enough data to emit 'readable' even though state.length === 0 instead of issuing _read(). Which led to the TTY not recognizing that someone is waiting for the input.
PR-URL: #21690 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Ruben Bridgewater ruben@bridgewater.de
firass111 pushed a commit to firass111/Project_node1 that referenced this pull request
Avoid trying to emit 'readable' due to the fact that state.length is always >= state.highWaterMark if highWaterMark is 0. Therefore upon .read(0) call (through .on('readable')) stream assumed that it has enough data to emit 'readable' even though state.length === 0 instead of issuing _read(). Which led to the TTY not recognizing that someone is waiting for the input.
Fixes: nodejs/node#20503 Refs: nodejs/node#18372
PR-URL: nodejs/node#21690 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Ruben Bridgewater ruben@bridgewater.de
Labels
Issues and PRs related to the stream subsystem.