From 9bd2e07e0a70647a0bd62c6c4baf5f2563159479 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 14 Mar 2016 17:11:07 -0700 Subject: [PATCH 1/5] zlib: do not emit event on *Sync() methods WIP right now, still need to do it for more methods than just gunzipSync(). Refs: https://github.com/nodejs/node/issues/1668 --- lib/zlib.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/zlib.js b/lib/zlib.js index 0028c35d7923eb..1ad83add211168 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -453,7 +453,9 @@ Zlib.prototype.flush = function(kind, callback) { } }; -Zlib.prototype.close = function(callback) { +Zlib.prototype.close = function(callback, options) { + options = options || {}; + if (callback) process.nextTick(callback); @@ -464,7 +466,9 @@ Zlib.prototype.close = function(callback) { this._handle.close(); - process.nextTick(emitCloseNT, this); + if (!options.sync) { + process.nextTick(emitCloseNT, this); + } }; function emitCloseNT(self) { @@ -535,12 +539,12 @@ Zlib.prototype._processChunk = function(chunk, flushFlag, cb) { } if (nread >= kMaxLength) { - this.close(); + this.close(null, {sync: true}); throw new RangeError(kRangeErrorMessage); } var buf = Buffer.concat(buffers, nread); - this.close(); + this.close(null, {sync: true}); return buf; } From 2b9b3c3bc601c49443e3114334b7f7f8dec94f13 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 16 Mar 2016 17:06:21 -0700 Subject: [PATCH 2/5] fixup: add test --- test/parallel/test-zlib-sync-no-event.js | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 test/parallel/test-zlib-sync-no-event.js diff --git a/test/parallel/test-zlib-sync-no-event.js b/test/parallel/test-zlib-sync-no-event.js new file mode 100644 index 00000000000000..7d36249055b8f3 --- /dev/null +++ b/test/parallel/test-zlib-sync-no-event.js @@ -0,0 +1,9 @@ +'use strict'; +require('../common'); +const zlib = require('zlib'); + +const zipper = new zlib.Gzip(); +zipper.on('close', () => { throw new Error('unexpected `close` event'); }); + +const buffer = new Buffer('hello world'); +zipper._processChunk(buffer, zlib.Z_FINISH); From 2a86be94b40052abcb03ff9e080035c5a2063af3 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 18 Mar 2016 12:58:19 -0700 Subject: [PATCH 3/5] fixup: Object.assign() --- lib/zlib.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/zlib.js b/lib/zlib.js index 1ad83add211168..817f9b987afb4b 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -454,7 +454,7 @@ Zlib.prototype.flush = function(kind, callback) { }; Zlib.prototype.close = function(callback, options) { - options = options || {}; + options = Object.assign({sync: false}, options); if (callback) process.nextTick(callback); From eeedebaf4c466ec9a3c05efafeb34021903fb4b5 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 18 Mar 2016 20:52:25 -0700 Subject: [PATCH 4/5] fixup: no options object --- lib/zlib.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/zlib.js b/lib/zlib.js index 817f9b987afb4b..79c78ea4a51307 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -453,23 +453,22 @@ Zlib.prototype.flush = function(kind, callback) { } }; -Zlib.prototype.close = function(callback, options) { - options = Object.assign({sync: false}, options); +Zlib.prototype.close = function(callback) { + _close(this, callback); + process.nextTick(emitCloseNT, this); +}; +function _close(engine, callback) { if (callback) process.nextTick(callback); - if (this._closed) + if (engine._closed) return; - this._closed = true; - - this._handle.close(); + engine._closed = true; - if (!options.sync) { - process.nextTick(emitCloseNT, this); - } -}; + engine._handle.close(); +} function emitCloseNT(self) { self.emit('close'); @@ -539,12 +538,12 @@ Zlib.prototype._processChunk = function(chunk, flushFlag, cb) { } if (nread >= kMaxLength) { - this.close(null, {sync: true}); + _close(this); throw new RangeError(kRangeErrorMessage); } var buf = Buffer.concat(buffers, nread); - this.close(null, {sync: true}); + _close(this); return buf; } From a33a70a1266a012db976c08f9ceec08395f75631 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 18 Mar 2016 20:54:41 -0700 Subject: [PATCH 5/5] fixup: test improvement --- test/parallel/test-zlib-sync-no-event.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-zlib-sync-no-event.js b/test/parallel/test-zlib-sync-no-event.js index 7d36249055b8f3..81d1ad530de27c 100644 --- a/test/parallel/test-zlib-sync-no-event.js +++ b/test/parallel/test-zlib-sync-no-event.js @@ -1,9 +1,21 @@ 'use strict'; require('../common'); const zlib = require('zlib'); +const assert = require('assert'); + +const shouldNotBeCalled = () => { throw new Error('unexpected event'); }; + +const message = 'Come on, Fhqwhgads.'; const zipper = new zlib.Gzip(); -zipper.on('close', () => { throw new Error('unexpected `close` event'); }); +zipper.on('close', shouldNotBeCalled); + +const buffer = new Buffer(message); +const zipped = zipper._processChunk(buffer, zlib.Z_FINISH); + +const unzipper = new zlib.Gunzip(); +unzipper.on('close', shouldNotBeCalled); -const buffer = new Buffer('hello world'); -zipper._processChunk(buffer, zlib.Z_FINISH); +const unzipped = unzipper._processChunk(zipped, zlib.Z_FINISH); +assert.notEqual(zipped.toString(), message); +assert.strictEqual(unzipped.toString(), message);