Skip to content

Commit

Permalink
tls: fix SecurePair external memory reporting
Browse files Browse the repository at this point in the history
Ensure that AdjustAmountOfExternalAllocatedMemory() is called when
the SecurePair is destroyed.  Not doing so is not an actual memory
leak but it makes `process.memoryUsage().external` wildly inaccurate
and can cause performance problems due to excessive garbage collection.

PR-URL: #11896
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
bnoordhuis authored and MylesBorins committed Mar 28, 2017
1 parent a64aa44 commit fd93622
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 5 deletions.
6 changes: 1 addition & 5 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3227,11 +3227,7 @@ void Connection::Start(const FunctionCallbackInfo<Value>& args) {
void Connection::Close(const FunctionCallbackInfo<Value>& args) {
Connection* conn;
ASSIGN_OR_RETURN_UNWRAP(&conn, args.Holder());

if (conn->ssl_ != nullptr) {
SSL_free(conn->ssl_);
conn->ssl_ = nullptr;
}
conn->DestroySSL();
}


Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-tls-securepair-leak.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Flags: --expose-gc --no-deprecation
'use strict';

const common = require('../common');
const assert = require('assert');

if (!common.hasCrypto) {
common.skip('missing crypto');
return;
}

const { createSecureContext } = require('tls');
const { createSecurePair } = require('_tls_legacy');

const before = process.memoryUsage().external;
{
const context = createSecureContext();
const options = {};
for (let i = 0; i < 1e4; i += 1)
createSecurePair(context, false, false, false, options).destroy();
}
global.gc();
const after = process.memoryUsage().external;

// It's not an exact science but a SecurePair grows .external by about 45 kB.
// Unless AdjustAmountOfExternalAllocatedMemory() is called on destruction,
// 10,000 instances make it grow by well over 400 MB. Allow for some slop
// because objects like buffers also affect the external limit.
assert(after - before < 25 << 20);

0 comments on commit fd93622

Please sign in to comment.