From e95cc8a6a3d767433973621c49d634c080a3363c Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 6 Dec 2017 05:29:29 +0100
Subject: [PATCH 1/3] tls: unconsume stream on destroy

When the TLS stream is destroyed for whatever reason,
we should unset all callbacks on the underlying transport
stream.

Fixes: https://github.com/nodejs/node/issues/17475
---
 src/tls_wrap.cc                               | 17 ++++++++--
 ...test-tls-transport-destroy-after-own-gc.js | 32 +++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-tls-transport-destroy-after-own-gc.js

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 3b899ea12d501d..8b28a33f098c99 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -101,6 +101,15 @@ TLSWrap::~TLSWrap() {
 #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
   sni_context_.Reset();
 #endif  // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
+
+  if (stream_ == nullptr)
+    return;
+  stream_->set_destruct_cb({ nullptr, nullptr });
+  stream_->set_after_write_cb({ nullptr, nullptr });
+  stream_->set_alloc_cb({ nullptr, nullptr });
+  stream_->set_read_cb({ nullptr, nullptr });
+  stream_->set_destruct_cb({ nullptr, nullptr });
+  stream_->Unconsume();
 }
 
 
@@ -564,12 +573,16 @@ uint32_t TLSWrap::UpdateWriteQueueSize(uint32_t write_queue_size) {
 
 
 int TLSWrap::ReadStart() {
-  return stream_->ReadStart();
+  if (stream_ != nullptr)
+    return stream_->ReadStart();
+  return 0;
 }
 
 
 int TLSWrap::ReadStop() {
-  return stream_->ReadStop();
+  if (stream_ != nullptr)
+    return stream_->ReadStop();
+  return 0;
 }
 
 
diff --git a/test/parallel/test-tls-transport-destroy-after-own-gc.js b/test/parallel/test-tls-transport-destroy-after-own-gc.js
new file mode 100644
index 00000000000000..4ea8da7ca2864d
--- /dev/null
+++ b/test/parallel/test-tls-transport-destroy-after-own-gc.js
@@ -0,0 +1,32 @@
+// Flags: --expose-gc
+'use strict';
+
+// Regression test for https://github.com/nodejs/node/issues/17475
+// Unfortunately, this tests only "works" reliably when checked with valgrind or
+// a similar tool.
+
+/* eslint-disable no-unused-vars */
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const { TLSSocket } = require('tls');
+const makeDuplexPair = require('../common/duplexpair');
+
+let { clientSide } = makeDuplexPair();
+
+let clientTLS = new TLSSocket(clientSide, { isServer: false });
+let clientTLSHandle = clientTLS._handle;
+
+setImmediate(() => {
+  clientTLS = null;
+  global.gc();
+  clientTLSHandle = null;
+  global.gc();
+  setImmediate(() => {
+    clientSide = null;
+    global.gc();
+  });
+});

From b7949b03ae068638de8b5925f9ce4827ef009d0b Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 12 Dec 2017 18:35:44 +0100
Subject: [PATCH 2/3] [squash] bridgear comment

---
 test/parallel/test-tls-transport-destroy-after-own-gc.js | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/test/parallel/test-tls-transport-destroy-after-own-gc.js b/test/parallel/test-tls-transport-destroy-after-own-gc.js
index 4ea8da7ca2864d..46f630982af643 100644
--- a/test/parallel/test-tls-transport-destroy-after-own-gc.js
+++ b/test/parallel/test-tls-transport-destroy-after-own-gc.js
@@ -5,19 +5,17 @@
 // Unfortunately, this tests only "works" reliably when checked with valgrind or
 // a similar tool.
 
-/* eslint-disable no-unused-vars */
-
 const common = require('../common');
 if (!common.hasCrypto)
   common.skip('missing crypto');
 
-const assert = require('assert');
 const { TLSSocket } = require('tls');
 const makeDuplexPair = require('../common/duplexpair');
 
 let { clientSide } = makeDuplexPair();
 
 let clientTLS = new TLSSocket(clientSide, { isServer: false });
+// eslint-disable-next-line no-unused-vars
 let clientTLSHandle = clientTLS._handle;
 
 setImmediate(() => {

From a95108878ee518cf51e3edbb0cd719369f3c407e Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 13 Dec 2017 04:07:56 +0100
Subject: [PATCH 3/3] [squash] reference test

---
 src/tls_wrap.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 8b28a33f098c99..3f5ed2c57580ff 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -102,6 +102,10 @@ TLSWrap::~TLSWrap() {
   sni_context_.Reset();
 #endif  // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
 
+  // See test/parallel/test-tls-transport-destroy-after-own-gc.js:
+  // If this TLSWrap is garbage collected, we cannot allow callbacks to be
+  // called on this stream.
+
   if (stream_ == nullptr)
     return;
   stream_->set_destruct_cb({ nullptr, nullptr });