Skip to content
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: allow writes after readable 'end' to finish #31082

Closed
wants to merge 3 commits into from

Conversation

@addaleax
Copy link
Member

addaleax commented Dec 24, 2019

Call the callback for writes that occur after the stream is closed.
This also requires changes to the code to not call .destroy()
on the stream in .on('end'), and to ignore chunks written
afterwards.

Previously, these writes would just queue up silently, as their
_write() callback would never have been called.

/cc @lpinca

Fixes: #30976

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Call the callback for writes that occur after the stream is closed.
This also requires changes to the code to not call `.destroy()`
on the stream in `.on('end')`, and to ignore chunks written
afterwards.

Previously, these writes would just queue up silently, as their
`_write()` callback would never have been called.

Fixes: #30976
lib/zlib.js Outdated Show resolved Hide resolved
Co-Authored-By: Denys Otrishko <9109612+lundibundi@users.noreply.github.com>
@nodejs-github-bot

This comment has been minimized.

@lpinca
lpinca approved these changes Dec 25, 2019
@lpinca

This comment has been minimized.

Copy link
Member

lpinca commented Dec 25, 2019

Thank you Anna.

lib/zlib.js Outdated Show resolved Hide resolved
Co-Authored-By: Ben Noordhuis <info@bnoordhuis.nl>
@nodejs-github-bot

This comment has been minimized.

@Trott
Trott approved these changes Dec 26, 2019
@nodejs-github-bot

This comment has been minimized.

@addaleax

This comment has been minimized.

Copy link
Member Author

addaleax commented Dec 27, 2019

Landed in 0e89b64

@addaleax addaleax closed this Dec 27, 2019
@addaleax addaleax deleted the addaleax:zlib-finish-writes branch Dec 27, 2019
addaleax added a commit that referenced this pull request Dec 27, 2019
Call the callback for writes that occur after the stream is closed.
This also requires changes to the code to not call `.destroy()`
on the stream in `.on('end')`, and to ignore chunks written
afterwards.

Previously, these writes would just queue up silently, as their
`_write()` callback would never have been called.

Fixes: #30976

PR-URL: #31082
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@lpinca

This comment has been minimized.

Copy link
Member

lpinca commented Dec 27, 2019

I think this should not land on v10.x unless 28db96f is also backported. Please correct me if I am wrong.

@addaleax

This comment has been minimized.

Copy link
Member Author

addaleax commented Dec 27, 2019

@lpinca I don’t think there’s a reason to not land this on v10.x and make behaviour more consistent across versions, even if 28db96f doesn’t land

BridgeAR added a commit that referenced this pull request Jan 3, 2020
Call the callback for writes that occur after the stream is closed.
This also requires changes to the code to not call `.destroy()`
on the stream in `.on('end')`, and to ignore chunks written
afterwards.

Previously, these writes would just queue up silently, as their
`_write()` callback would never have been called.

Fixes: #30976

PR-URL: #31082
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
targos added a commit that referenced this pull request Jan 14, 2020
Call the callback for writes that occur after the stream is closed.
This also requires changes to the code to not call `.destroy()`
on the stream in `.on('end')`, and to ignore chunks written
afterwards.

Previously, these writes would just queue up silently, as their
`_write()` callback would never have been called.

Fixes: #30976

PR-URL: #31082
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.