Skip to content

Commit 82c43ae

Browse files
committed
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 <info@bnoordhuis.nl> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent b2e20b0 commit 82c43ae

File tree

2 files changed

+42
-31
lines changed

2 files changed

+42
-31
lines changed

src/tls_wrap.cc

+19-7
Original file line numberDiff line numberDiff line change
@@ -280,17 +280,30 @@ void TLSWrap::EncOut() {
280280
&count);
281281
CHECK(write_size_ != 0 && count != 0);
282282

283+
uv_buf_t buf[arraysize(data)];
284+
uv_buf_t* bufs = buf;
285+
for (size_t i = 0; i < count; i++)
286+
buf[i] = uv_buf_init(data[i], size[i]);
287+
288+
int err = stream_->DoTryWrite(&bufs, &count);
289+
if (err != 0) {
290+
InvokeQueued(err);
291+
} else if (count == 0) {
292+
env()->SetImmediate([](Environment* env, void* data) {
293+
NODE_COUNT_NET_BYTES_SENT(write_size_);
294+
static_cast<TLSWrap*>(data)->OnStreamAfterWrite(nullptr, 0);
295+
}, this, object());
296+
return;
297+
}
298+
283299
Local<Object> req_wrap_obj =
284300
env()->write_wrap_constructor_function()
285301
->NewInstance(env()->context()).ToLocalChecked();
286302
WriteWrap* write_req = WriteWrap::New(env(),
287303
req_wrap_obj,
288304
static_cast<StreamBase*>(stream_));
289305

290-
uv_buf_t buf[arraysize(data)];
291-
for (size_t i = 0; i < count; i++)
292-
buf[i] = uv_buf_init(data[i], size[i]);
293-
int err = stream_->DoWrite(write_req, buf, count, nullptr);
306+
err = stream_->DoWrite(write_req, buf, count, nullptr);
294307

295308
// Ignore errors, this should be already handled in js
296309
if (err) {
@@ -303,9 +316,8 @@ void TLSWrap::EncOut() {
303316

304317

305318
void TLSWrap::OnStreamAfterWrite(WriteWrap* req_wrap, int status) {
306-
// We should not be getting here after `DestroySSL`, because all queued writes
307-
// must be invoked with UV_ECANCELED
308-
CHECK_NE(ssl_, nullptr);
319+
if (ssl_ == nullptr)
320+
status = UV_ECANCELED;
309321

310322
// Handle error
311323
if (status) {

test/async-hooks/test-writewrap.js

+23-24
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,20 @@
11
'use strict';
22

33
const common = require('../common');
4-
if (!common.hasCrypto)
5-
common.skip('missing crypto');
6-
74
const assert = require('assert');
85
const initHooks = require('./init-hooks');
9-
const fixtures = require('../common/fixtures');
106
const { checkInvocations } = require('./hook-checks');
11-
const tls = require('tls');
7+
const net = require('net');
128

139
const hooks = initHooks();
1410
hooks.enable();
1511

1612
//
1713
// Creating server and listening on port
1814
//
19-
const server = tls
20-
.createServer({
21-
cert: fixtures.readSync('test_cert.pem'),
22-
key: fixtures.readSync('test_key.pem')
23-
})
15+
const server = net.createServer()
2416
.on('listening', common.mustCall(onlistening))
25-
.on('secureConnection', common.mustCall(onsecureConnection))
17+
.on('connection', common.mustCall(onconnection))
2618
.listen(0);
2719

2820
assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0);
@@ -32,16 +24,17 @@ function onlistening() {
3224
//
3325
// Creating client and connecting it to server
3426
//
35-
tls
36-
.connect(server.address().port, { rejectUnauthorized: false })
37-
.on('secureConnect', common.mustCall(onsecureConnect));
27+
net
28+
.connect(server.address().port)
29+
.on('connect', common.mustCall(onconnect));
3830

3931
assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0);
4032
}
4133

4234
function checkDestroyedWriteWraps(n, stage) {
4335
const as = hooks.activitiesOfTypes('WRITEWRAP');
44-
assert.strictEqual(as.length, n, `${n} WRITEWRAPs when ${stage}`);
36+
assert.strictEqual(as.length, n,
37+
`${as.length} out of ${n} WRITEWRAPs when ${stage}`);
4538

4639
function checkValidWriteWrap(w) {
4740
assert.strictEqual(w.type, 'WRITEWRAP');
@@ -53,41 +46,47 @@ function checkDestroyedWriteWraps(n, stage) {
5346
as.forEach(checkValidWriteWrap);
5447
}
5548

56-
function onsecureConnection() {
49+
function onconnection(conn) {
50+
conn.resume();
5751
//
5852
// Server received client connection
5953
//
60-
checkDestroyedWriteWraps(3, 'server got secure connection');
54+
checkDestroyedWriteWraps(0, 'server got connection');
6155
}
6256

63-
function onsecureConnect() {
57+
function onconnect() {
6458
//
6559
// Client connected to server
6660
//
67-
checkDestroyedWriteWraps(4, 'client connected');
61+
checkDestroyedWriteWraps(0, 'client connected');
6862

6963
//
7064
// Destroying client socket
7165
//
72-
this.destroy();
66+
this.write('f'.repeat(128000), () => onafterwrite(this));
67+
}
68+
69+
function onafterwrite(self) {
70+
checkDestroyedWriteWraps(1, 'client destroyed');
71+
self.destroy();
7372

74-
checkDestroyedWriteWraps(4, 'client destroyed');
73+
checkDestroyedWriteWraps(1, 'client destroyed');
7574

7675
//
7776
// Closing server
7877
//
7978
server.close(common.mustCall(onserverClosed));
80-
checkDestroyedWriteWraps(4, 'server closing');
79+
checkDestroyedWriteWraps(1, 'server closing');
8180
}
8281

8382
function onserverClosed() {
84-
checkDestroyedWriteWraps(4, 'server closed');
83+
checkDestroyedWriteWraps(1, 'server closed');
8584
}
8685

8786
process.on('exit', onexit);
8887

8988
function onexit() {
9089
hooks.disable();
9190
hooks.sanityCheck('WRITEWRAP');
92-
checkDestroyedWriteWraps(4, 'process exits');
91+
checkDestroyedWriteWraps(1, 'process exits');
9392
}

0 commit comments

Comments
 (0)