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: align with streams #32220

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions lib/zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const {
} = require('internal/util/types');
const binding = internalBinding('zlib');
const assert = require('internal/assert');
const finished = require('internal/streams/end-of-stream');
const {
Buffer,
kMaxLength
Expand Down Expand Up @@ -173,14 +174,13 @@ function zlibOnError(message, errno, code) {
const self = this[owner_symbol];
// There is no way to cleanly recover.
// Continuing only obscures problems.
_close(self);
self._hadError = true;

// eslint-disable-next-line no-restricted-syntax
const error = new Error(message);
error.errno = errno;
error.code = code;
self.emit('error', error);
self.destroy(error);
self._error = error;
ronag marked this conversation as resolved.
Show resolved Hide resolved
}

// 1. Returns false for undefined and NaN
Expand Down Expand Up @@ -260,8 +260,8 @@ function ZlibBase(opts, mode, handle, { flush, finishFlush, fullFlush }) {
}
}

Transform.call(this, { autoDestroy: false, ...opts });
this._hadError = false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could keep _hadError for compat if necessary.

Transform.call(this, { autoDestroy: true, ...opts });
this._error = null;
this.bytesWritten = 0;
this._handle = handle;
handle[owner_symbol] = this;
Expand All @@ -274,7 +274,6 @@ function ZlibBase(opts, mode, handle, { flush, finishFlush, fullFlush }) {
this._defaultFlushFlag = flush;
this._finishFlushFlag = finishFlush;
this._defaultFullFlushFlag = fullFlush;
this.once('end', this.close);
this._info = opts && opts.info;
}
ObjectSetPrototypeOf(ZlibBase.prototype, Transform.prototype);
Expand Down Expand Up @@ -368,7 +367,7 @@ ZlibBase.prototype.flush = function(kind, callback) {
};

ZlibBase.prototype.close = function(callback) {
_close(this, callback);
if (callback) finished(this, callback);
this.destroy();
};

Expand Down Expand Up @@ -432,6 +431,8 @@ function processChunkSync(self, chunk, flushFlag) {
availOutBefore); // out_len
if (error)
throw error;
else if (self._error)
throw self._error;

availOutAfter = state[0];
availInAfter = state[1];
Expand Down Expand Up @@ -512,7 +513,7 @@ function processCallback() {
const self = this[owner_symbol];
const state = self._writeState;

if (self._hadError || self.destroyed) {
if (self.destroyed) {
this.buffer = null;
this.cb();
return;
Expand Down Expand Up @@ -578,10 +579,7 @@ function processCallback() {
this.cb();
}

function _close(engine, callback) {
if (callback)
process.nextTick(callback);

function _close(engine) {
// Caller may invoke .close after a zlib error (which will null _handle).
if (!engine._handle)
return;
Expand Down Expand Up @@ -678,7 +676,7 @@ ObjectSetPrototypeOf(Zlib, ZlibBase);
function paramsAfterFlushCallback(level, strategy, callback) {
assert(this._handle, 'zlib binding closed');
this._handle.params(level, strategy);
if (!this._hadError) {
if (!this.destroyed) {
this._level = level;
this._strategy = strategy;
if (callback) callback();
Expand Down
26 changes: 22 additions & 4 deletions test/parallel/test-zlib-destroy.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,31 @@
'use strict';

require('../common');
const common = require('../common');

const assert = require('assert');
const zlib = require('zlib');

// Verify that the zlib transform does clean up
// the handle when calling destroy.

const ts = zlib.createGzip();
ts.destroy();
assert.strictEqual(ts._handle, null);
{
const ts = zlib.createGzip();
ts.destroy();
assert.strictEqual(ts._handle, null);

ts.on('close', common.mustCall(() => {
ts.close(common.mustCall());
}));
}

{
// Ensure 'error' is only emitted once.
const decompress = zlib.createGunzip(15);

decompress.on('error', common.mustCall((err) => {
decompress.close();
}));

decompress.write('something invalid');
decompress.destroy(new Error('asd'));
}