tls: fix SecurePair external memory reporting

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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>

Author: bnoordhuis

src/node_crypto.cc

@@ -3248,11 +3248,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();
 }
 
 

test/parallel/test-tls-securepair-leak.js

@@ -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);