mirror of
https://github.com/nodejs/node.git
synced 2025-05-09 13:05:07 +00:00
tls: fix leak of WriteWrap+TLSWrap combination
Writing data to TLSWrap instance during handshake will result in it being queued in `write_item_queue_`. This queue won't get cleared up until the end of the handshake. Technically, it gets cleared on `~TLSWrap` invocation, however this won't ever happen because every `WriteWrap` holds a reference to the `TLSWrap` through JS object, meaning that they are doomed to be alive for eternity. To breach this dreadful contract a knight shall embark from the `close` function to kill the dragon of memory leak with his magic spear of `destroySSL`. `destroySSL` cleans up `write_item_queue_` and frees `SSL` structure, both are good for memory usage. PR-URL: https://github.com/nodejs/node/pull/9586 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit is contained in:
parent
a804627837
commit
478fabf3e4
@ -305,14 +305,31 @@ proxiedMethods.forEach(function(name) {
|
||||
});
|
||||
|
||||
tls_wrap.TLSWrap.prototype.close = function close(cb) {
|
||||
if (this.owner)
|
||||
let ssl;
|
||||
if (this.owner) {
|
||||
ssl = this.owner.ssl;
|
||||
this.owner.ssl = null;
|
||||
}
|
||||
|
||||
// Invoke `destroySSL` on close to clean up possibly pending write requests
|
||||
// that may self-reference TLSWrap, leading to leak
|
||||
const done = () => {
|
||||
if (ssl) {
|
||||
ssl.destroySSL();
|
||||
if (ssl._secureContext.singleUse) {
|
||||
ssl._secureContext.context.close();
|
||||
ssl._secureContext.context = null;
|
||||
}
|
||||
}
|
||||
if (cb)
|
||||
cb();
|
||||
};
|
||||
|
||||
if (this._parentWrap && this._parentWrap._handle === this._parent) {
|
||||
this._parentWrap.once('close', cb);
|
||||
this._parentWrap.once('close', done);
|
||||
return this._parentWrap.destroy();
|
||||
}
|
||||
return this._parent.close(cb);
|
||||
return this._parent.close(done);
|
||||
};
|
||||
|
||||
TLSSocket.prototype._wrapHandle = function(wrap) {
|
||||
|
26
test/parallel/test-tls-writewrap-leak.js
Normal file
26
test/parallel/test-tls-writewrap-leak.js
Normal file
@ -0,0 +1,26 @@
|
||||
'use strict';
|
||||
const common = require('../common');
|
||||
|
||||
if (!common.hasCrypto) {
|
||||
common.skip('missing crypto');
|
||||
return;
|
||||
}
|
||||
|
||||
const assert = require('assert');
|
||||
const net = require('net');
|
||||
const tls = require('tls');
|
||||
|
||||
const server = net.createServer(common.mustCall((c) => {
|
||||
c.destroy();
|
||||
})).listen(0, common.mustCall(() => {
|
||||
const c = tls.connect({ port: server.address().port });
|
||||
c.on('error', () => {
|
||||
// Otherwise `.write()` callback won't be invoked.
|
||||
c.destroyed = false;
|
||||
});
|
||||
|
||||
c.write('hello', common.mustCall((err) => {
|
||||
assert.equal(err.code, 'ECANCELED');
|
||||
server.close();
|
||||
}));
|
||||
}));
|
Loading…
Reference in New Issue
Block a user