diff --git a/HISTORY.md b/HISTORY.md index 1fd9761..1e233c1 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,8 @@ +unreleased +========== + + * Add Zlib steam support and Node.js leak work around + 1.0.4 / 2016-01-15 ================== diff --git a/README.md b/README.md index da5f8a9..496b529 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,12 @@ Destroy the given stream. In most cases, this is identical to a simple and add a listener to the `open` event to call `stream.close()` if it is fired. This is for a Node.js bug that will leak a file descriptor if `.destroy()` is called before `open`. - 2. If the `stream` is not an instance of `Stream`, then nothing happens. - 3. If the `stream` has a `.destroy()` method, then call it. + 2. If the `stream` is an instance of a zlib stream, then call `stream.destroy()` + and close the underlying zlib handle if open, otherwise call `stream.close()`. + This is for consistency across Node.js versions and a Node.js bug that will + leak a native zlib handle. + 3. If the `stream` is not an instance of `Stream`, then nothing happens. + 4. If the `stream` has a `.destroy()` method, then call it. The function returns the `stream` passed in as the argument. diff --git a/index.js b/index.js index 223e41e..d330ae5 100644 --- a/index.js +++ b/index.js @@ -13,6 +13,7 @@ var ReadStream = require('fs').ReadStream var Stream = require('stream') +var Zlib = require('zlib') /** * Module exports. @@ -33,6 +34,16 @@ function destroy (stream) { return destroyReadStream(stream) } + if (stream instanceof Zlib.Gzip || + stream instanceof Zlib.Gunzip || + stream instanceof Zlib.Deflate || + stream instanceof Zlib.DeflateRaw || + stream instanceof Zlib.Inflate || + stream instanceof Zlib.InflateRaw || + stream instanceof Zlib.Unzip) { + return destroyZlibStream(stream) + } + if (!(stream instanceof Stream)) { return stream } @@ -62,6 +73,65 @@ function destroyReadStream (stream) { return stream } +/** + * Destroy a Zlib stream. + * + * Zlib streams don't have a destroy function in Node.js 6. On top of that + * simply calling destroy on a zlib stream in Node.js 8+ will result in a + * memory leak. So until that is fixed, we need to call both close AND destroy. + * + * PR to fix memory leak: https://github.com/nodejs/node/pull/23734 + * + * In Node.js 6+8, it's important that destroy is called before close as the + * stream would otherwise emit the error 'zlib binding closed'. + * + * @param {object} stream + * @private + */ + +function destroyZlibStream (stream) { + if (typeof stream.destroy === 'function') { + // node.js core bug work-around + // istanbul ignore if: node.js 0.8 + if (stream._binding) { + // node.js < 0.10.0 + stream.destroy() + if (stream._processing) { + stream._needDrain = true + stream.once('drain', onDrainClearBinding) + } else { + stream._binding.clear() + } + } else if (stream._destroy && stream._destroy !== Stream.Transform.prototype._destroy) { + // node.js >= 12, ^11.1.0, ^10.15.1 + stream.destroy() + } else if (stream._destroy && typeof stream.close === 'function') { + // node.js 7, 8 + stream.destroyed = true + stream.close() + } else { + // fallback + // istanbul ignore next + stream.destroy() + } + } else if (typeof stream.close === 'function') { + // node.js < 8 fallback + stream.close() + } + + return stream +} + +/** + * On drain handler to clear binding. + * @private + */ + +// istanbul ignore next: node.js 0.8 +function onDrainClearBinding () { + this._binding.clear() +} + /** * On open handler to close stream. * @private diff --git a/test/test.js b/test/test.js index 569f638..529f504 100644 --- a/test/test.js +++ b/test/test.js @@ -2,6 +2,7 @@ var assert = require('assert') var fs = require('fs') var net = require('net') +var zlib = require('zlib') var destroy = require('..') @@ -72,9 +73,31 @@ describe('destroy', function () { }) }) }) + + ;['Gzip', 'Gunzip', 'Deflate', 'DeflateRaw', 'Inflate', 'InflateRaw', 'Unzip'].forEach(function (type) { + var method = 'create' + type + + describe('Zlib.' + type, function () { + it('should destroy a zlib stream', function () { + var stream = zlib[method]() + assert(!isdestroyed(stream)) + destroy(stream) + assert(isdestroyed(stream)) + }) + + it('should destroy a zlib stream after write', function () { + var stream = zlib[method]() + assert(!isdestroyed(stream)) + stream.write('') + destroy(stream) + assert(isdestroyed(stream)) + }) + }) + }) }) function isdestroyed (stream) { - // readable for 0.8, destroyed for 0.10+ - return stream.readable === false || stream.destroyed === true + // readable for 0.8, destroyed for 0.10+, _closed for zlib < 8 + return stream.readable === false || stream.destroyed === true || + stream._closed === true }