From 87cef63ccbbc3db875c4d4e98c42d90623c2ed0e Mon Sep 17 00:00:00 2001
From: Matteo Collina <hello@matteocollina.com>
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.

PR-URL: https://github.com/nodejs/node/pull/13156
Fixes: https://github.com/websockets/ws/issues/1118
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
---
 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);
+  }));
+}