XXX write before destroy can fail after-the-fact

This is a bug, it isn't supposed to happen, but how to prevent it?

When TLS finishes the DoWrite the underlying stream is closed, so the
stream_resource()->Write() in EncOut() fails with write-after-destroy.

See test/parallel/test-tls-destroy-stream.js, and its inline comments.

Author: sam-github

lib/net.js

@@ -581,6 +581,7 @@ Socket.prototype._destroy = function(exception, cb) {
     this[kBytesRead] = this._handle.bytesRead;
     this[kBytesWritten] = this._handle.bytesWritten;
 
+    // process._rawDebug('handle.close', this._handle.close+'');
     this._handle.close(() => {
       debug('emit close');
       this.emit('close', isException);

src/js_stream.cc

@@ -153,6 +153,7 @@ void JSStream::New(const FunctionCallbackInfo<Value>& args) {
 
 template <class Wrap>
 void JSStream::Finish(const FunctionCallbackInfo<Value>& args) {
+  // finishShutdown?
   CHECK(args[0]->IsObject());
   Wrap* w = static_cast<Wrap*>(StreamReq::FromObject(args[0].As<Object>()));
 

src/node_crypto.cc

@@ -2732,6 +2732,9 @@ void SSLWrap<Base>::CertCbDone(const FunctionCallbackInfo<Value>& args) {
 
 template <class Base>
 void SSLWrap<Base>::DestroySSL() {
+  fprintf(stderr, "%s SSLWrap::DestroySSL() ssl? %d\n",
+      is_server() ? "server" : "client", !!ssl_);
+
   if (!ssl_)
     return;
 

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

@@ -9,7 +9,7 @@ const net = require('net');
 const assert = require('assert');
 const tls = require('tls');
 
-tls.DEFAULT_MAX_VERSION = 'TLSv1.3';
+tls.DEFAULT_MAX_VERSION = 'TLSv1.3';  // XXX Switch to 1.2 to see old behaviour
 
 // This test ensures that an instance of StreamWrap should emit "end" and
 // "close" when the socket on the other side call `destroy()` instead of
@@ -18,25 +18,42 @@ tls.DEFAULT_MAX_VERSION = 'TLSv1.3';
 const CONTENT = 'Hello World';
 const tlsServer = tls.createServer(
   {
+    maxVersion: 'TLSv1.3', // passes with 1.2
     key: fixtures.readSync('test_key.pem'),
     cert: fixtures.readSync('test_cert.pem'),
     ca: [fixtures.readSync('test_ca.pem')],
   },
   (socket) => {
-    console.log('tls server connection');
-    //socket.on('error', common.mustNotCall(console.log));
-    socket.on('close', common.mustCall());
-    socket.write(CONTENT);
+    process._rawDebug('server on secureConnection', socket.getProtocol());
+    socket.on('error', (err) => {
+      process._rawDebug('server on error:', err);
+      // process.reallyExit(7);
+      //XXX assert.ifError(err);
+    });
+    socket.on('close', common.mustCall((hadError) => {
+      process._rawDebug('server on close: hadError?', hadError);
+      // XXX? assert.ok(hadError);
+    }));
+    // XXX destroy() tears down socket before CONTENT gets flushed. probably
+    // because of key update messages... confirm this with tracing. Is it
+    // a bug in the test?
+    process._rawDebug('server socket.write()');
+    socket.write(CONTENT, (err) => {
+      process._rawDebug('server write cb, err:', err);
+      // process.reallyExit(9);
+    });
+    process._rawDebug('server socket.destroy()');
     socket.destroy();
+    //process.reallyExit(3);
   },
 );
 
 const server = net.createServer((conn) => {
-  console.log('net server connection');
+  process._rawDebug('net server connection');
   conn.on('error', common.mustNotCall());
   // Assume that we want to use data to determine what to do with connections.
   conn.once('data', common.mustCall((chunk) => {
-    console.log('net conn data');
+    process._rawDebug('net conn data');
     const { clientSide, serverSide } = makeDuplexPair();
     serverSide.on('close', common.mustCall(() => {
       conn.destroy();
@@ -52,7 +69,7 @@ const server = net.createServer((conn) => {
     }));
 
     process.nextTick(() => {
-      console.log('net conn unshift data');
+      process._rawDebug('net conn unshift data');
       conn.unshift(chunk);
     });