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>
This commit is contained in:
Anna Henningsen 2018-01-28 17:26:08 +01:00
parent b2e20b002b
commit 82c43aed16
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
2 changed files with 42 additions and 31 deletions

View File

@ -280,6 +280,22 @@ void TLSWrap::EncOut() {
&count); &count);
CHECK(write_size_ != 0 && count != 0); 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 = Local<Object> req_wrap_obj =
env()->write_wrap_constructor_function() env()->write_wrap_constructor_function()
->NewInstance(env()->context()).ToLocalChecked(); ->NewInstance(env()->context()).ToLocalChecked();
@ -287,10 +303,7 @@ void TLSWrap::EncOut() {
req_wrap_obj, req_wrap_obj,
static_cast<StreamBase*>(stream_)); static_cast<StreamBase*>(stream_));
uv_buf_t buf[arraysize(data)]; err = stream_->DoWrite(write_req, buf, count, nullptr);
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);
// Ignore errors, this should be already handled in js // Ignore errors, this should be already handled in js
if (err) { if (err) {
@ -303,9 +316,8 @@ void TLSWrap::EncOut() {
void TLSWrap::OnStreamAfterWrite(WriteWrap* req_wrap, int status) { void TLSWrap::OnStreamAfterWrite(WriteWrap* req_wrap, int status) {
// We should not be getting here after `DestroySSL`, because all queued writes if (ssl_ == nullptr)
// must be invoked with UV_ECANCELED status = UV_ECANCELED;
CHECK_NE(ssl_, nullptr);
// Handle error // Handle error
if (status) { if (status) {

View File

@ -1,14 +1,10 @@
'use strict'; 'use strict';
const common = require('../common'); const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert'); const assert = require('assert');
const initHooks = require('./init-hooks'); const initHooks = require('./init-hooks');
const fixtures = require('../common/fixtures');
const { checkInvocations } = require('./hook-checks'); const { checkInvocations } = require('./hook-checks');
const tls = require('tls'); const net = require('net');
const hooks = initHooks(); const hooks = initHooks();
hooks.enable(); hooks.enable();
@ -16,13 +12,9 @@ hooks.enable();
// //
// Creating server and listening on port // Creating server and listening on port
// //
const server = tls const server = net.createServer()
.createServer({
cert: fixtures.readSync('test_cert.pem'),
key: fixtures.readSync('test_key.pem')
})
.on('listening', common.mustCall(onlistening)) .on('listening', common.mustCall(onlistening))
.on('secureConnection', common.mustCall(onsecureConnection)) .on('connection', common.mustCall(onconnection))
.listen(0); .listen(0);
assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0); assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0);
@ -32,16 +24,17 @@ function onlistening() {
// //
// Creating client and connecting it to server // Creating client and connecting it to server
// //
tls net
.connect(server.address().port, { rejectUnauthorized: false }) .connect(server.address().port)
.on('secureConnect', common.mustCall(onsecureConnect)); .on('connect', common.mustCall(onconnect));
assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0); assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0);
} }
function checkDestroyedWriteWraps(n, stage) { function checkDestroyedWriteWraps(n, stage) {
const as = hooks.activitiesOfTypes('WRITEWRAP'); 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) { function checkValidWriteWrap(w) {
assert.strictEqual(w.type, 'WRITEWRAP'); assert.strictEqual(w.type, 'WRITEWRAP');
@ -53,35 +46,41 @@ function checkDestroyedWriteWraps(n, stage) {
as.forEach(checkValidWriteWrap); as.forEach(checkValidWriteWrap);
} }
function onsecureConnection() { function onconnection(conn) {
conn.resume();
// //
// Server received client connection // Server received client connection
// //
checkDestroyedWriteWraps(3, 'server got secure connection'); checkDestroyedWriteWraps(0, 'server got connection');
} }
function onsecureConnect() { function onconnect() {
// //
// Client connected to server // Client connected to server
// //
checkDestroyedWriteWraps(4, 'client connected'); checkDestroyedWriteWraps(0, 'client connected');
// //
// Destroying client socket // Destroying client socket
// //
this.destroy(); this.write('f'.repeat(128000), () => onafterwrite(this));
}
checkDestroyedWriteWraps(4, 'client destroyed'); function onafterwrite(self) {
checkDestroyedWriteWraps(1, 'client destroyed');
self.destroy();
checkDestroyedWriteWraps(1, 'client destroyed');
// //
// Closing server // Closing server
// //
server.close(common.mustCall(onserverClosed)); server.close(common.mustCall(onserverClosed));
checkDestroyedWriteWraps(4, 'server closing'); checkDestroyedWriteWraps(1, 'server closing');
} }
function onserverClosed() { function onserverClosed() {
checkDestroyedWriteWraps(4, 'server closed'); checkDestroyedWriteWraps(1, 'server closed');
} }
process.on('exit', onexit); process.on('exit', onexit);
@ -89,5 +88,5 @@ process.on('exit', onexit);
function onexit() { function onexit() {
hooks.disable(); hooks.disable();
hooks.sanityCheck('WRITEWRAP'); hooks.sanityCheck('WRITEWRAP');
checkDestroyedWriteWraps(4, 'process exits'); checkDestroyedWriteWraps(1, 'process exits');
} }