Skip to content

Commit

Permalink
Merge de9c24b into feb759f
Browse files Browse the repository at this point in the history
  • Loading branch information
watson committed Oct 23, 2018
2 parents feb759f + de9c24b commit 4175685
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 2 deletions.
8 changes: 6 additions & 2 deletions README.md
Expand Up @@ -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.
1. If the `stream` is not an instance of `Stream`, then nothing happens.
1. 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.
1. If the `stream` has a `.destroy()` method, then call it.

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

Expand Down
34 changes: 34 additions & 0 deletions index.js
Expand Up @@ -12,6 +12,7 @@
*/

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

/**
Expand All @@ -37,13 +38,46 @@ 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()
if (typeof stream.close === 'function') stream.close()

return stream
}

/**
* Destroy a ReadStream.
*
Expand Down
36 changes: 36 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,41 @@ describe('destroy', function () {
})
})
})

describe('zlib', function () {
var version = process.versions.node.split('.').map(function (n) {
return Number(n)
})
var preNode0_10 = version[0] === 0 && version[1] < 10
var types = ['Gzip', 'Gunzip', 'Deflate', 'DeflateRaw', 'Inflate', 'InflateRaw', 'Unzip']

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

it('should destroy a ' + type + ' stream created sync', function () {
var stream = zlib[constructor]()
destroy(stream)
if (preNode0_10) {
assert.strictEqual(stream._ended, true)
} else {
assert.strictEqual(stream._closed, true)
}
})

it('should destroy a ' + type + ' stream created async', function (done) {
var stream = zlib[constructor]()
setTimeout(function () {
destroy(stream)
if (preNode0_10) {
assert.strictEqual(stream._ended, true)
} else {
assert.strictEqual(stream._closed, true)
}
done()
}, 0)
})
})
})
})

function isdestroyed (stream) {
Expand Down

0 comments on commit 4175685

Please sign in to comment.