-
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
zlib: check if the stream is destroyed before push #14330
Conversation
If the stream is destroyed while the transform is still being applied, push() should not be called, and the internal state should be cleared. See: koajs/compress#60
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 if the CI comes back OK.
CI is green. CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/910/ (cc @refack) |
Junkins is still in a funky mood, might need to re-run later... nodejs/build#798 |
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/915/ cc @refack can you have a look? Or is still problematic? |
Landing, CITGM seems ok. |
Landed as aa496f4. |
If the stream is destroyed while the transform is still being applied, push() should not be called, and the internal state should be cleared. See: koajs/compress#60 PR-URL: #14330 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Maybe add |
@refack koa-compress tests were passing. |
Will they add a regression test for this? |
I couldn't reproduce this programmatically with koa-compress, but only when load-testing or when my machine was under severe load. |
If the stream is destroyed while the transform is still being applied, push() should not be called, and the internal state should be cleared. Refs: koajs/compress#60 PR-URL: nodejs#14330 Backport-PR-URL: nodejs#14396 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Conflicts: lib/zlib.js
@mcollina should this be backported? It would need to be done manually |
This depends on #12925, which is flagged as semver-major. So, no. |
If the stream is destroyed while the transform is still being
applied, push() should not be called, and the internal state
should be cleared.
See: koajs/compress#60
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
zlib, stream