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>
This commit is contained in:
parent
ba10ea8f3a
commit
b53473f0e7
30
lib/zlib.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
|
||||
|
@ -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…
x
Reference in New Issue
Block a user