tls: emit errors on close whilst async action

lib/_tls_wrap.js

@@ -61,6 +61,9 @@ function loadSession(self, hello, cb) {
     if (err)
       return cb(err);
 
+    if (!self._handle)
+      return cb(new Error('Socket is closed'));
+
     // NOTE: That we have disabled OpenSSL's internal session storage in
     // `node_crypto.cc` and hence its safe to rely on getting servername only
     // from clienthello or this place.
@@ -91,6 +94,9 @@ function loadSNI(self, servername, cb) {
     if (err)
       return cb(err);
 
+    if (!self._handle)
+      return cb(new Error('Socket is closed'));
+
     // TODO(indutny): eventually disallow raw `SecureContext`
     if (context)
       self._handle.sni_context = context.context || context;
@@ -127,6 +133,9 @@ function requestOCSP(self, hello, ctx, cb) {
     if (err)
       return cb(err);
 
+    if (!self._handle)
+      return cb(new Error('Socket is closed'));
+
     if (response)
       self._handle.setOCSPResponse(response);
     cb(null);
@@ -157,6 +166,9 @@ function oncertcb(info) {
       if (err)
         return self.destroy(err);
 
+      if (!self._handle)
+        return cb(new Error('Socket is closed'));
+
       self._handle.certCbDone();
     });
   });
@@ -179,6 +191,9 @@ function onnewsession(key, session) {
       return;
     once = true;
 
+    if (!self._handle)
+      return cb(new Error('Socket is closed'));
+
     self._handle.newSessionDone();
 
     self._newSessionPending = false;
@@ -290,13 +305,18 @@ TLSSocket.prototype._wrapHandle = function(handle) {
   });
 
   this.on('close', function() {
-    this._destroySSL();
+    // Make sure we are not doing it on OpenSSL's stack
+    setImmediate(destroySSL, this);
     res = null;
   });
 
   return res;
 };
 
+function destroySSL(self) {
+  self._destroySSL();
+}
+
 TLSSocket.prototype._destroySSL = function _destroySSL() {
   if (!this.ssl) return;
   this.ssl.destroySSL();

src/tls_wrap.cc

@@ -86,12 +86,6 @@ TLSWrap::~TLSWrap() {
   sni_context_.Reset();
 #endif  // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
 
-  // Move all writes to pending
-  MakePending();
-
-  // And destroy
-  InvokeQueued(UV_ECANCELED);
-
   ClearError();
 }
 
@@ -337,6 +331,9 @@ void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) {
     return;
   }
 
+  if (wrap->ssl_ == nullptr)
+    return;
+
   // Commit
   NodeBIO::FromBIO(wrap->enc_out_)->Read(nullptr, wrap->write_size_);
 
@@ -554,6 +551,7 @@ int TLSWrap::DoWrite(WriteWrap* w,
                      size_t count,
                      uv_stream_t* send_handle) {
   CHECK_EQ(send_handle, nullptr);
+  CHECK_NE(ssl_, nullptr);
 
   bool empty = true;
 
@@ -627,6 +625,11 @@ void TLSWrap::OnAfterWriteImpl(WriteWrap* w, void* ctx) {
 void TLSWrap::OnAllocImpl(size_t suggested_size, uv_buf_t* buf, void* ctx) {
   TLSWrap* wrap = static_cast<TLSWrap*>(ctx);
 
+  if (wrap->ssl_ == nullptr) {
+    *buf = uv_buf_init(nullptr, 0);
+    return;
+  }
+
   size_t size = 0;
   buf->base = NodeBIO::FromBIO(wrap->enc_in_)->PeekWritable(&size);
   buf->len = size;
@@ -747,6 +750,10 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo<Value>& args) {
 void TLSWrap::EnableSessionCallbacks(
     const FunctionCallbackInfo<Value>& args) {
   TLSWrap* wrap = Unwrap<TLSWrap>(args.Holder());
+  if (wrap->ssl_ == nullptr) {
+    return wrap->env()->ThrowTypeError(
+        "EnableSessionCallbacks after destroySSL");
+  }
   wrap->enable_session_callbacks();
   NodeBIO::FromBIO(wrap->enc_in_)->set_initial(kMaxHelloLength);
   wrap->hello_parser_.Start(SSLWrap<TLSWrap>::OnClientHello,
@@ -757,7 +764,16 @@ void TLSWrap::EnableSessionCallbacks(
 
 void TLSWrap::DestroySSL(const FunctionCallbackInfo<Value>& args) {
   TLSWrap* wrap = Unwrap<TLSWrap>(args.Holder());
+
+  // Move all writes to pending
+  wrap->MakePending();
+
+  // And destroy
+  wrap->InvokeQueued(UV_ECANCELED);
+
+  // Destroy the SSL structure and friends
   wrap->SSLWrap<TLSWrap>::DestroySSL();
+
   delete wrap->clear_in_;
   wrap->clear_in_ = nullptr;
 }

test/parallel/test-tls-async-cb-after-socket-end.js

@@ -0,0 +1,73 @@
+'use strict';
+
+var common = require('../common');
+
+var assert = require('assert');
+var path = require('path');
+var fs = require('fs');
+var constants = require('constants');
+
+if (!common.hasCrypto) {
+  console.log('1..0 # Skipped: missing crypto');
+  process.exit();
+}
+
+var tls = require('tls');
+
+var options = {
+  secureOptions: constants.SSL_OP_NO_TICKET,
+  key: fs.readFileSync(path.join(common.fixturesDir, 'test_key.pem')),
+  cert: fs.readFileSync(path.join(common.fixturesDir, 'test_cert.pem'))
+};
+
+var server = tls.createServer(options, function(c) {
+});
+
+var sessionCb = null;
+var client = null;
+
+server.on('newSession', function(key, session, done) {
+  done();
+});
+
+server.on('resumeSession', function(id, cb) {
+  sessionCb = cb;
+
+  next();
+});
+
+server.listen(1443, function() {
+  var clientOpts = {
+    port: 1443,
+    rejectUnauthorized: false,
+    session: false
+  };
+
+  var s1 = tls.connect(clientOpts, function() {
+    clientOpts.session = s1.getSession();
+    console.log('1st secure');
+
+    s1.destroy();
+    var s2 = tls.connect(clientOpts, function(s) {
+      console.log('2nd secure');
+
+      s2.destroy();
+    }).on('connect', function() {
+      console.log('2nd connected');
+      client = s2;
+
+      next();
+    });
+  });
+});
+
+function next() {
+  if (!client || !sessionCb)
+    return;
+
+  client.destroy();
+  setTimeout(function() {
+    sessionCb();
+    server.close();
+  }, 100);
+}

test/parallel/test-tls-js-stream.js

@@ -61,6 +61,7 @@ var server = tls.createServer({
 
     socket.end('hello');
     socket.resume();
+    socket.destroy();
   });
 
   socket.once('close', function() {