From 17aebdf445402e25a2be9b51fff64d6c8a7e590f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 22 May 2017 18:03:55 +0200 Subject: [PATCH] stream: fix destroy(err, cb) regression Fixed a regression that caused the callback passed to destroy() to not be called if the stream was already destroyed. This caused a regression on the ws module in CITGM introduced by https://github.com/nodejs/node/pull/12925. Fixes: https://github.com/websockets/ws/issues/1118 --- lib/internal/streams/destroy.js | 5 ++++- test/parallel/test-net-socket-destroy-send.js | 22 +++++++++++++++++++ test/parallel/test-stream-readable-destroy.js | 14 ++++++++++++ test/parallel/test-stream-writable-destroy.js | 15 +++++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-net-socket-destroy-send.js diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js index 18a2a839843bb9..37e7d4bc364299 100644 --- a/lib/internal/streams/destroy.js +++ b/lib/internal/streams/destroy.js @@ -8,7 +8,10 @@ function destroy(err, cb) { this._writableState.destroyed; if (readableDestroyed || writableDestroyed) { - if (err && (!this._writableState || !this._writableState.errorEmitted)) { + if (cb) { + cb(err); + } else if (err && + (!this._writableState || !this._writableState.errorEmitted)) { process.nextTick(emitErrorNT, this, err); } return; diff --git a/test/parallel/test-net-socket-destroy-send.js b/test/parallel/test-net-socket-destroy-send.js new file mode 100644 index 00000000000000..309e41f6c94e6d --- /dev/null +++ b/test/parallel/test-net-socket-destroy-send.js @@ -0,0 +1,22 @@ +'use strict'; + +const common = require('../common'); +const net = require('net'); +const assert = require('assert'); + +const server = net.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + const conn = net.createConnection(port); + + conn.on('connect', common.mustCall(function() { + conn.destroy(); + conn.on('error', common.mustCall(function(err) { + assert.strictEqual(err.message, 'This socket is closed'); + })); + conn.write(Buffer.from('kaboom'), common.mustCall(function(err) { + assert.strictEqual(err.message, 'This socket is closed'); + })); + server.close(); + })); +})); diff --git a/test/parallel/test-stream-readable-destroy.js b/test/parallel/test-stream-readable-destroy.js index 800b6be0865377..def20d26c34080 100644 --- a/test/parallel/test-stream-readable-destroy.js +++ b/test/parallel/test-stream-readable-destroy.js @@ -160,3 +160,17 @@ const { inherits } = require('util'); new MyReadable(); } + +{ + // destroy and destroy callback + const read = new Readable({ + read() {} + }); + read.resume(); + + const expected = new Error('kaboom'); + + read.destroy(expected, common.mustCall(function(err) { + assert.strictEqual(expected, err); + })); +} diff --git a/test/parallel/test-stream-writable-destroy.js b/test/parallel/test-stream-writable-destroy.js index a91f148f9e5cd5..87e55eccc3f6fd 100644 --- a/test/parallel/test-stream-writable-destroy.js +++ b/test/parallel/test-stream-writable-destroy.js @@ -170,3 +170,18 @@ const { inherits } = require('util'); new MyWritable(); } + +{ + // destroy and destroy callback + const write = new Writable({ + write(chunk, enc, cb) { cb(); } + }); + + write.destroy(); + + const expected = new Error('kaboom'); + + write.destroy(expected, common.mustCall(function(err) { + assert.strictEqual(expected, err); + })); +}