Skip to content

Commit

Permalink
tls: unconsume stream on destroy
Browse files Browse the repository at this point in the history
When the TLS stream is destroyed for whatever reason,
we should unset all callbacks on the underlying transport
stream.

Fixes: #17475
  • Loading branch information
addaleax committed Dec 12, 2017
1 parent 7a055f1 commit e95cc8a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
17 changes: 15 additions & 2 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ TLSWrap::~TLSWrap() {
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
sni_context_.Reset();
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB

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();
}


Expand Down Expand Up @@ -564,12 +573,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;
}


Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-tls-transport-destroy-after-own-gc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// 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.

/* eslint-disable no-unused-vars */

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const { TLSSocket } = require('tls');
const makeDuplexPair = require('../common/duplexpair');

let { clientSide } = makeDuplexPair();

let clientTLS = new TLSSocket(clientSide, { isServer: false });
let clientTLSHandle = clientTLS._handle;

setImmediate(() => {
clientTLS = null;
global.gc();
clientTLSHandle = null;
global.gc();
setImmediate(() => {
clientSide = null;
global.gc();
});
});

0 comments on commit e95cc8a

Please sign in to comment.