Browse Source

zlib: remove `_closed` in source

This is purely cleanup and carries no visible behavioural changes.
Up to now, `this._closed` was used in zlib.js as a
synonym of `!this._handle`. This change makes this connection
explicit and removes the `_closed` property from zlib streams,
as the previous duplication has been the cause of subtle errors
like https://github.com/nodejs/node/issues/6034.

This also makes zlib errors lead to an explicit `_close()` call
rather than waiting for garbage collection to clean up the handle,
thus returning memory resources earlier in the case of an error.

Add a getter for `_closed` so that the property remains accessible
by legacy code.

PR-URL: https://github.com/nodejs/node/pull/6574
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
v6.x
Anna Henningsen 9 years ago
committed by Rod Vagg
parent
commit
64415564de
  1. 30
      lib/zlib.js
  2. 2
      test/parallel/test-zlib-close-after-error.js

30
lib/zlib.js

@ -364,7 +364,7 @@ function Zlib(opts, mode) {
this._handle.onerror = function(message, errno) {
// there is no way to cleanly recover.
// continuing only obscures problems.
self._handle = null;
_close(self);
self._hadError = true;
var error = new Error(message);
@ -387,11 +387,16 @@ function Zlib(opts, mode) {
this._buffer = Buffer.allocUnsafe(this._chunkSize);
this._offset = 0;
this._closed = false;
this._level = level;
this._strategy = strategy;
this.once('end', this.close);
Object.defineProperty(this, '_closed', {
get: () => { return !this._handle; },
configurable: true,
enumerable: true
});
}
util.inherits(Zlib, Transform);
@ -412,7 +417,7 @@ Zlib.prototype.params = function(level, strategy, callback) {
if (this._level !== level || this._strategy !== strategy) {
var self = this;
this.flush(binding.Z_SYNC_FLUSH, function() {
assert(!self._closed, 'zlib binding closed');
assert(self._handle, 'zlib binding closed');
self._handle.params(level, strategy);
if (!self._hadError) {
self._level = level;
@ -426,7 +431,7 @@ Zlib.prototype.params = function(level, strategy, callback) {
};
Zlib.prototype.reset = function() {
assert(!this._closed, 'zlib binding closed');
assert(this._handle, 'zlib binding closed');
return this._handle.reset();
};
@ -469,15 +474,12 @@ function _close(engine, callback) {
if (callback)
process.nextTick(callback);
if (engine._closed)
// Caller may invoke .close after a zlib error (which will null _handle).
if (!engine._handle)
return;
engine._closed = true;
// Caller may invoke .close after a zlib error (which will null _handle).
if (engine._handle) {
engine._handle.close();
}
engine._handle.close();
engine._handle = null;
}
function emitCloseNT(self) {
@ -493,7 +495,7 @@ Zlib.prototype._transform = function(chunk, encoding, cb) {
if (chunk !== null && !(chunk instanceof Buffer))
return cb(new Error('invalid input'));
if (this._closed)
if (!this._handle)
return cb(new Error('zlib binding closed'));
// If it's the last chunk, or a final flush, we use the Z_FINISH flush flag
@ -533,7 +535,7 @@ Zlib.prototype._processChunk = function(chunk, flushFlag, cb) {
error = er;
});
assert(!this._closed, 'zlib binding closed');
assert(this._handle, 'zlib binding closed');
do {
var res = this._handle.writeSync(flushFlag,
chunk, // in
@ -559,7 +561,7 @@ Zlib.prototype._processChunk = function(chunk, flushFlag, cb) {
return buf;
}
assert(!this._closed, 'zlib binding closed');
assert(this._handle, 'zlib binding closed');
var req = this._handle.write(flushFlag,
chunk, // in
inOff, // in_off

2
test/parallel/test-zlib-close-after-error.js

@ -8,7 +8,9 @@ const zlib = require('zlib');
const decompress = zlib.createGunzip(15);
decompress.on('error', common.mustCall((err) => {
assert.strictEqual(decompress._closed, true);
assert.doesNotThrow(() => decompress.close());
}));
assert.strictEqual(decompress._closed, false);
decompress.write('something invalid');

Loading…
Cancel
Save