Skip to content
This repository

zlib: emit "close" if Z_STREAM_END occurs. #3747

Closed
wants to merge 2 commits into from

4 participants

Yi EungJun Isaac Z. Schlueter Nodejs Jenkins Fedor Indutny
Yi EungJun
npcode commented July 20, 2012

In src/node_zlib.cc, Z_OK, Z_STREAM_END and Z_BUF_ERROR has been ignored because they are considered normal statuses.

But Z_STREAM_END must be handled. Even if zlib does not end when it meets Z_STREAM_END, it should at least tell user so that user can stop it.

(Actaully, I believe "end" is better than "close" for this case, but emitting "end" may cause exception because zlib does not allow to write something to itself if zlib._ended is true.)

Isaac Z. Schlueter
Collaborator
isaacs commented July 20, 2012

This isn't quite accurate.

Z_STREAM_END is the return value if all the output is consumed, and the flush param was set to Z_FINISH. However, the "close" event should be used only when the underlying resource is completely disposed of (ie, when deflateEnd or inflateEnd has been successfully called to free the memory.) This would occur when the binding has been deleted, but we don't actually track that, and just let that happen when V8's garbage collector cleans things up.

However, upon receiving a Z_STREAM_END, you can call .reset() on the stream, and then re-use it again. In fact, this is a useful way to prevent having to create new underlying objects.

What we probably ought to do is remove the place where we're currently emitting "end", and only emit "end" when a Z_STREAM_END return value is encountered. Would you like to change your patch to do that?

Yi EungJun zlib: Emit "end" instead of "close" for Z_STREAM_END
Emit "end" instead of "close" when Z_STREAM_END return value is
encountered. And remove emitting "end" in Zlib.end().
3efc532
Yi EungJun
npcode commented July 20, 2012

You are right, @isaacs . I have done as you said and it looks to work well.

Nodejs Jenkins
Collaborator

Can one of the admins verify this patch?

Fedor Indutny
Collaborator

I guess it isn't relevant anymore?

Fedor Indutny indutny closed this February 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Jul 21, 2012
Yi EungJun zlib: emit "close" if Z_STREAM_END occurs. 45cb346
Yi EungJun zlib: Emit "end" instead of "close" for Z_STREAM_END
Emit "end" instead of "close" when Z_STREAM_END return value is
encountered. And remove emitting "end" in Zlib.end().
3efc532
This page is out of date. Refresh to see the latest.
7  lib/zlib.js
@@ -346,7 +346,6 @@ Zlib.prototype.end = function end(chunk, cb) {
346 346
   var self = this;
347 347
   this._ending = true;
348 348
   var ret = this.write(chunk, function() {
349  
-    self.emit('end');
350 349
     if (cb) cb();
351 350
   });
352 351
   this._ended = true;
@@ -392,7 +391,7 @@ Zlib.prototype._process = function() {
392 391
   req.callback = callback;
393 392
   this._processing = req;
394 393
 
395  
-  function callback(availInAfter, availOutAfter, buffer) {
  394
+  function callback(err, availInAfter, availOutAfter, buffer) {
396 395
     if (self._hadError) return;
397 396
 
398 397
     var have = availOutBefore - availOutAfter;
@@ -438,6 +437,10 @@ Zlib.prototype._process = function() {
438 437
     self._processing = false;
439 438
     if (cb) cb();
440 439
     self._process();
  440
+
  441
+    if (err == binding.Z_STREAM_END) {
  442
+      self.emit('end');
  443
+    }
441 444
   }
442 445
 };
443 446
 
3  src/node_zlib.cc
@@ -211,7 +211,8 @@ class ZCtx : public ObjectWrap {
211 211
     // call the write() cb
212 212
     assert(ctx->handle_->Get(callback_sym)->IsFunction() &&
213 213
            "Invalid callback");
214  
-    Local<Value> args[2] = { avail_in, avail_out };
  214
+    Local<Value> args[3] = { Local<Value>::New(Number::New(ctx->err_)),
  215
+                             avail_in, avail_out };
215 216
     MakeCallback(ctx->handle_, callback_sym, ARRAY_SIZE(args), args);
216 217
 
217 218
     ctx->Unref();
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.