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: #18676
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: addaleax

src/tls_wrap.cc

@@ -287,17 +287,30 @@ 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();
   WriteWrap* write_req = WriteWrap::New(env(),
                                         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) {

test/async-hooks/test-writewrap.js

@@ -1,28 +1,20 @@
 '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();
 
 //
 // 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,41 +46,47 @@ 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);
 
 function onexit() {
   hooks.disable();
   hooks.sanityCheck('WRITEWRAP');
-  checkDestroyedWriteWraps(4, 'process exits');
+  checkDestroyedWriteWraps(1, 'process exits');
 }