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 streams emitting data after end #2485

Closed
jhamhader opened this issue Aug 21, 2015 · 4 comments
Closed

zlib streams emitting data after end #2485

jhamhader opened this issue Aug 21, 2015 · 4 comments
Labels
zlib Issues and PRs related to the zlib subsystem.

Comments

@jhamhader
Copy link
Contributor

Hey,

I noticed that zlib streams still emit data (i.e in deflate stream, it writes the adler32 checksum) after .end() has been called.
Here is a test (with useful prints, obviously should be removed later) which fails on the recent 3.0.x:
(notice the data written after "deflate end cb")

var zlib = require('zlib');

var input = 'Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa.'
var deflate = zlib.createDeflate();
var alreadyEnded = 0;

deflate.on('data', function(chunk) {
        console.log('deflate data:');
        console.log(chunk);
        if (alreadyEnded) {
                throw "Stream already ended"
        }   
});

deflate.on('prefinish', function() {
        console.log('deflate prefinish event');
});

deflate.on('finish', function() {
        console.log('deflate finish event');
});

deflate.write(input);
defalte.flush(function() {
        deflate.end(function () {
                console.log('deflate end cb');
                alreadyEnded = 1;
        });
});

I would really like to help fix this issue and it to be my first contribution here, so if anyone is willing to help with mentoring, that will be great.
From what I understand, calling end() triggers calling endWritable() which calls prefinish().
Transform's _flush() is registered to the prefinish event. Problem is, _flush() calls an async zlib Write(). Meanwhile, endWritable continues and calls end()'s callback.

Am I on to an issue?
Should zlib's _flush write synchronously if this is the last write?

@mscdex mscdex added the zlib Issues and PRs related to the zlib subsystem. label Aug 21, 2015
@thefourtheye
Copy link
Contributor

Cc @chrisdickinson?

@ChALkeR
Copy link
Member

ChALkeR commented Aug 23, 2015

Could be related: #1668.

@chrisdickinson
Copy link
Contributor

It's a little confusing, but end() doesn't directly correlate to the 'end' event in transform streams. Instead, it corresponds to 'finish' on the writable side of the stream. That is, once the data has been written and processed by to the stream's backing resource, it will emit "prefinish", then "finish". "prefinish", which most interests us at this point, is fired the moment we get the end call. "prefinish" triggers the _flush machinery in Transform streams, which in turn, triggers zlib's _flush, which will force zlib to spit out the crc trailer as a "data" event and call the callback, which calls code in Transform that ends the readable side of the stream. Note: the "end" event won't be fired until the data is pulled out of the readable side of the buffer — in this case, the .on('data') listener is doing the pulling — so we end up with:

1. "prefinish"
  1. _flush                              1. <writable cbs are exhausted>
  2. zlib._flush                         2. "finish"
  3. zlib pushes empty buffer            3. user-supplied end() cb called
  4. <time passes>
  5. zlib emits trailer info
  6. zlib ends stream with `push(null)`
  7. stream emits "data"
  8. stream emits "end"

Data isn't appearing after the "end" event, it's appearing between "finish" and "end" — which is an okay thing for data to do. It is pretty confusing that end()'s callback fires on "finish", though.

Hopefully this clarifies what's going on in the above example!

(I've omitted the manual .flush call, since that's not part of the streams spec, and wouldn't affect the outcome.)

@jhamhader
Copy link
Contributor Author

Might be a bit confusing but seems like there is no real issue here. Maybe for transform streams register the callback to end() on the 'end' event?
Thanks for the detailed clarification @chrisdickinson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants