1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

tls: unconsume stream on destroy

Browse Source 
When the TLS stream is destroyed for whatever reason,
we should unset all callbacks on the underlying transport
stream.

PR-URL: https://github.com/nodejs/node/pull/17478
Fixes: https://github.com/nodejs/node/issues/17475
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Anna Henningsen 2017-12-06 05:29:29 +01:00
parent cd71fc1545
commit f96a86cac5
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
2 changed files with 49 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/tls_wrap.cc
View File

@ -101,6 +101,19 @@ TLSWrap::~TLSWrap() {
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
sni_context_.Reset();
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
// See test/parallel/test-tls-transport-destroy-after-own-gc.js:
// If this TLSWrap is garbage collected, we cannot allow callbacks to be
// called on this stream.
if (stream_ == nullptr)
return;
stream_->set_destruct_cb({ nullptr, nullptr });
stream_->set_after_write_cb({ nullptr, nullptr });
stream_->set_alloc_cb({ nullptr, nullptr });
stream_->set_read_cb({ nullptr, nullptr });
stream_->set_destruct_cb({ nullptr, nullptr });
stream_->Unconsume();
}
@ -564,12 +577,16 @@ uint32_t TLSWrap::UpdateWriteQueueSize(uint32_t write_queue_size) {
int TLSWrap::ReadStart() {
return stream_->ReadStart();
if (stream_ != nullptr)
return stream_->ReadStart();
return 0;
}
int TLSWrap::ReadStop() {
return stream_->ReadStop();
if (stream_ != nullptr)
return stream_->ReadStop();
return 0;
}

30
test/parallel/test-tls-transport-destroy-after-own-gc.js Normal file
View File

@ -0,0 +1,30 @@
// Flags: --expose-gc
'use strict';
// Regression test for https://github.com/nodejs/node/issues/17475
// Unfortunately, this tests only "works" reliably when checked with valgrind or
// a similar tool.
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const { TLSSocket } = require('tls');
const makeDuplexPair = require('../common/duplexpair');
let { clientSide } = makeDuplexPair();
let clientTLS = new TLSSocket(clientSide, { isServer: false });
// eslint-disable-next-line no-unused-vars
let clientTLSHandle = clientTLS._handle;
setImmediate(() => {
clientTLS = null;
global.gc();
clientTLSHandle = null;
global.gc();
setImmediate(() => {
clientSide = null;
global.gc();
});
});