Skip to content

Commit

Permalink
Merge 91a27b6 into d1a76c6
Browse files Browse the repository at this point in the history
  • Loading branch information
watson committed Oct 23, 2018
2 parents d1a76c6 + 91a27b6 commit 9f2a959
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 1 deletion.
6 changes: 5 additions & 1 deletion README.md
Expand Up @@ -27,7 +27,11 @@ Destroy the given stream. In most cases, this is identical to a simple
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.
3. If the `stream` is a zlib stream, it will first call
`stream.destroy()` (if that function exists), followed by
`stream.close()` as that will correctly release the zlib handle from
memory.
4. If the `stream` has a `.destroy()` method, then call it.

The function returns the `stream` passed in as the argument.

Expand Down
44 changes: 44 additions & 0 deletions index.js
Expand Up @@ -13,6 +13,7 @@

var ReadStream = require('fs').ReadStream
var Stream = require('stream')
var zlib = require('zlib')

/**
* Module exports.
Expand All @@ -37,8 +38,51 @@ function destroy (stream) {
return 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 (typeof stream.destroy === 'function') {
stream.destroy()
}

return stream
}

/**
* Destroy a Zlib stream.
*
* Zlib streams doesn'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') {
stream.destroy()

// node.js core bug work-around
if (stream._handle) {
stream._handle.close()
} else if (stream._binding) {
stream._binding.close()
}
} else if (typeof stream.close === 'function') {
stream.close()
}

return stream
Expand Down
33 changes: 33 additions & 0 deletions test/test.js
Expand Up @@ -2,6 +2,7 @@
var assert = require('assert')
var fs = require('fs')
var net = require('net')
var zlib = require('zlib')

var destroy = require('..')

Expand Down Expand Up @@ -72,6 +73,38 @@ describe('destroy', function () {
})
})
})

describe('Zlib', function () {
var types = ['Gzip', 'Gunzip', 'Deflate', 'DeflateRaw', 'Inflate', 'InflateRaw', 'Unzip']

types.forEach(function (type) {
var method = 'create' + type

describe('.' + type, function () {
describe('when call sync', function () {
it('should destory stream', function (done) {
var stream = zlib[method]()
stream.on('close', function () {
done()
})
destroy(stream)
})
})

describe('when call async', function () {
it('should destroy stream', function (done) {
var stream = zlib[method]()
stream.on('close', function () {
done()
})
setTimeout(function () {
destroy(stream)
}, 10)
})
})
})
})
})
})

function isdestroyed (stream) {
Expand Down

0 comments on commit 9f2a959

Please sign in to comment.