From efb46465392153963b656cb55d2eda149a050285 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sun, 28 Jan 2018 17:26:08 +0100
Subject: [PATCH] tls_wrap: use DoTryWrite()

Use `DoTryWrite()` to write data to the underlying socket.
This does probably not make any difference in performance
because the callback is still deferred (for now), but
brings TLSWrap in line with other things that write to
streams.

PR-URL: https://github.com/nodejs/node/pull/18676
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/tls_wrap.cc                    | 26 ++++++++++++-----
 test/async-hooks/test-writewrap.js | 47 +++++++++++++++---------------
 2 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index a38aa7a4c3484a..561a9785f28edc 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -287,6 +287,22 @@ void TLSWrap::EncOut() {
                                                                  &count);
   CHECK(write_size_ != 0 && count != 0);
 
+  uv_buf_t buf[arraysize(data)];
+  uv_buf_t* bufs = buf;
+  for (size_t i = 0; i < count; i++)
+    buf[i] = uv_buf_init(data[i], size[i]);
+
+  int err = stream_->DoTryWrite(&bufs, &count);
+  if (err != 0) {
+    InvokeQueued(err);
+  } else if (count == 0) {
+    env()->SetImmediate([](Environment* env, void* data) {
+      NODE_COUNT_NET_BYTES_SENT(write_size_);
+      static_cast<TLSWrap*>(data)->OnStreamAfterWrite(nullptr, 0);
+    }, this, object());
+    return;
+  }
+
   Local<Object> req_wrap_obj =
       env()->write_wrap_constructor_function()
           ->NewInstance(env()->context()).ToLocalChecked();
@@ -294,10 +310,7 @@ void TLSWrap::EncOut() {
                                         req_wrap_obj,
                                         static_cast<StreamBase*>(stream_));
 
-  uv_buf_t buf[arraysize(data)];
-  for (size_t i = 0; i < count; i++)
-    buf[i] = uv_buf_init(data[i], size[i]);
-  int err = stream_->DoWrite(write_req, buf, count, nullptr);
+  err = stream_->DoWrite(write_req, buf, count, nullptr);
 
   // Ignore errors, this should be already handled in js
   if (err) {
@@ -310,9 +323,8 @@ void TLSWrap::EncOut() {
 
 
 void TLSWrap::OnStreamAfterWrite(WriteWrap* req_wrap, int status) {
-  // We should not be getting here after `DestroySSL`, because all queued writes
-  // must be invoked with UV_ECANCELED
-  CHECK_NE(ssl_, nullptr);
+  if (ssl_ == nullptr)
+    status = UV_ECANCELED;
 
   // Handle error
   if (status) {
diff --git a/test/async-hooks/test-writewrap.js b/test/async-hooks/test-writewrap.js
index d349f635665ddd..ed17887684c9c2 100644
--- a/test/async-hooks/test-writewrap.js
+++ b/test/async-hooks/test-writewrap.js
@@ -1,14 +1,10 @@
 'use strict';
 
 const common = require('../common');
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-
 const assert = require('assert');
 const initHooks = require('./init-hooks');
-const fixtures = require('../common/fixtures');
 const { checkInvocations } = require('./hook-checks');
-const tls = require('tls');
+const net = require('net');
 
 const hooks = initHooks();
 hooks.enable();
@@ -16,13 +12,9 @@ hooks.enable();
 //
 // Creating server and listening on port
 //
-const server = tls
-  .createServer({
-    cert: fixtures.readSync('test_cert.pem'),
-    key: fixtures.readSync('test_key.pem')
-  })
+const server = net.createServer()
   .on('listening', common.mustCall(onlistening))
-  .on('secureConnection', common.mustCall(onsecureConnection))
+  .on('connection', common.mustCall(onconnection))
   .listen(0);
 
 assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0);
@@ -32,16 +24,17 @@ function onlistening() {
   //
   // Creating client and connecting it to server
   //
-  tls
-    .connect(server.address().port, { rejectUnauthorized: false })
-    .on('secureConnect', common.mustCall(onsecureConnect));
+  net
+    .connect(server.address().port)
+    .on('connect', common.mustCall(onconnect));
 
   assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0);
 }
 
 function checkDestroyedWriteWraps(n, stage) {
   const as = hooks.activitiesOfTypes('WRITEWRAP');
-  assert.strictEqual(as.length, n, `${n} WRITEWRAPs when ${stage}`);
+  assert.strictEqual(as.length, n,
+                     `${as.length} out of ${n} WRITEWRAPs when ${stage}`);
 
   function checkValidWriteWrap(w) {
     assert.strictEqual(w.type, 'WRITEWRAP');
@@ -53,35 +46,41 @@ function checkDestroyedWriteWraps(n, stage) {
   as.forEach(checkValidWriteWrap);
 }
 
-function onsecureConnection() {
+function onconnection(conn) {
+  conn.resume();
   //
   // Server received client connection
   //
-  checkDestroyedWriteWraps(3, 'server got secure connection');
+  checkDestroyedWriteWraps(0, 'server got connection');
 }
 
-function onsecureConnect() {
+function onconnect() {
   //
   // Client connected to server
   //
-  checkDestroyedWriteWraps(4, 'client connected');
+  checkDestroyedWriteWraps(0, 'client connected');
 
   //
   // Destroying client socket
   //
-  this.destroy();
+  this.write('f'.repeat(128000), () => onafterwrite(this));
+}
+
+function onafterwrite(self) {
+  checkDestroyedWriteWraps(1, 'client destroyed');
+  self.destroy();
 
-  checkDestroyedWriteWraps(4, 'client destroyed');
+  checkDestroyedWriteWraps(1, 'client destroyed');
 
   //
   // Closing server
   //
   server.close(common.mustCall(onserverClosed));
-  checkDestroyedWriteWraps(4, 'server closing');
+  checkDestroyedWriteWraps(1, 'server closing');
 }
 
 function onserverClosed() {
-  checkDestroyedWriteWraps(4, 'server closed');
+  checkDestroyedWriteWraps(1, 'server closed');
 }
 
 process.on('exit', onexit);
@@ -89,5 +88,5 @@ process.on('exit', onexit);
 function onexit() {
   hooks.disable();
   hooks.sanityCheck('WRITEWRAP');
-  checkDestroyedWriteWraps(4, 'process exits');
+  checkDestroyedWriteWraps(1, 'process exits');
 }