From 9289374248deeaf761bbbf560fd536902629ecec Mon Sep 17 00:00:00 2001
From: ywave620 <60539365+ywave620@users.noreply.github.com>
Date: Tue, 26 Nov 2024 11:25:22 +0800
Subject: [PATCH] http2: fix memory leak caused by premature listener removing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Http2Session should always call ondone into JS to detach the
handle. In some case, ondone is defered to be called by the
StreamListener through WriteWrap, we should be careful of this
before getting rid of the StreamListener.

PR-URL: https://github.com/nodejs/node/pull/55966
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
---
 src/node_http2.cc                             |  4 +-
 ...-h2leak-destroy-session-on-socket-ended.js | 80 +++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-h2leak-destroy-session-on-socket-ended.js

diff --git a/src/node_http2.cc b/src/node_http2.cc
index 593c8b5f07a2a4..888f70bd4df8a3 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -803,13 +803,15 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
     CHECK_EQ(nghttp2_session_terminate_session(session_.get(), code), 0);
     SendPendingData();
   } else if (stream_ != nullptr) {
+    // so that the previous listener of the socket, typically, JS code of a
+    // (tls) socket will be notified of any activity later
     stream_->RemoveStreamListener(this);
   }
 
   set_destroyed();
 
   // If we are writing we will get to make the callback in OnStreamAfterWrite.
-  if (!is_write_in_progress()) {
+  if (!is_write_in_progress() || !stream_) {
     Debug(this, "make done session callback");
     HandleScope scope(env()->isolate());
     MakeCallback(env()->ondone_string(), 0, nullptr);
diff --git a/test/parallel/test-h2leak-destroy-session-on-socket-ended.js b/test/parallel/test-h2leak-destroy-session-on-socket-ended.js
new file mode 100644
index 00000000000000..3f0fe3e69d924d
--- /dev/null
+++ b/test/parallel/test-h2leak-destroy-session-on-socket-ended.js
@@ -0,0 +1,80 @@
+'use strict';
+// Flags: --expose-gc
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const http2 = require('http2');
+const tls = require('tls');
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+
+const registry = new FinalizationRegistry(common.mustCall((name) => {
+  assert(name, 'session');
+}));
+
+const server = http2.createSecureServer({
+  key: fixtures.readKey('agent1-key.pem'),
+  cert: fixtures.readKey('agent1-cert.pem'),
+});
+
+let firstServerStream;
+
+
+server.on('secureConnection', (s) => {
+  console.log('secureConnection');
+  s.on('end', () => {
+    console.log(s.destroyed); // false !!
+    s.destroy();
+    firstServerStream.session.destroy();
+
+    firstServerStream = null;
+
+    setImmediate(() => {
+      global.gc();
+      global.gc();
+
+      server.close();
+    });
+  });
+});
+
+server.on('session', (s) => {
+  registry.register(s, 'session');
+});
+
+server.on('stream', (stream) => {
+  console.log('stream...');
+  stream.write('a'.repeat(1024));
+  firstServerStream = stream;
+  setImmediate(() => console.log('Draining setImmediate after writing'));
+});
+
+
+server.listen(() => {
+  client();
+});
+
+
+const h2fstStream = [
+  'UFJJICogSFRUUC8yLjANCg0KU00NCg0K',
+  // http message (1st stream:)
+  'AAAABAAAAAAA',
+  'AAAPAQUAAAABhIJBiqDkHROdCbjwHgeG',
+];
+function client() {
+  const client = tls.connect({
+    port: server.address().port,
+    host: 'localhost',
+    rejectUnauthorized: false,
+    ALPNProtocols: ['h2']
+  }, () => {
+    client.end(Buffer.concat(h2fstStream.map((s) => Buffer.from(s, 'base64'))), (err) => {
+      assert.ifError(err);
+    });
+  });
+
+  client.on('error', (error) => {
+    console.error('Connection error:', error);
+  });
+}