Skip to content

Commit

Permalink
tls_wrap: use DoTryWrite()
Browse files Browse the repository at this point in the history
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: nodejs#18676
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and MayaLekova committed May 8, 2018
1 parent 51ce779 commit 9c04bc6
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 31 deletions.
26 changes: 19 additions & 7 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,17 +280,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) {
Expand All @@ -303,9 +316,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) {
Expand Down
47 changes: 23 additions & 24 deletions test/async-hooks/test-writewrap.js
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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');
Expand All @@ -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');
}

0 comments on commit 9c04bc6

Please sign in to comment.