From 9d93926e151c44f551bcf0e7a49369e6e0d5d47b Mon Sep 17 00:00:00 2001
From: theanarkh <theratliter@gmail.com>
Date: Sun, 24 Jul 2022 22:06:24 +0800
Subject: [PATCH] http: fix http server connection list when close

PR-URL: https://github.com/nodejs/node/pull/43949
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
---
 lib/_http_common.js                           |  1 +
 src/node_http_parser.cc                       | 15 ++++--
 ...-http-server-connection-list-when-close.js | 47 +++++++++++++++++++
 3 files changed, 58 insertions(+), 5 deletions(-)
 create mode 100644 test/parallel/test-http-server-connection-list-when-close.js

diff --git a/lib/_http_common.js b/lib/_http_common.js
index 654eadfe67d2e3..526901cb5f9681 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -182,6 +182,7 @@ function freeParser(parser, req, socket) {
     if (parser._consumed)
       parser.unconsume();
     cleanParser(parser);
+    parser.remove();
     if (parsers.free(parser) === false) {
       // Make sure the parser's stack has unwound before deleting the
       // corresponding C++ object through .close().
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 52c8e9e2589dd4..620d608713d31d 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -548,17 +548,21 @@ class Parser : public AsyncWrap, public StreamListener {
     Parser* parser;
     ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
 
-    if (parser->connectionsList_ != nullptr) {
-      parser->connectionsList_->Pop(parser);
-      parser->connectionsList_->PopActive(parser);
-    }
-
     // Since the Parser destructor isn't going to run the destroy() callbacks
     // it needs to be triggered manually.
     parser->EmitTraceEventDestroy();
     parser->EmitDestroy();
   }
 
+  static void Remove(const FunctionCallbackInfo<Value>& args) {
+    Parser* parser;
+    ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
+
+    if (parser->connectionsList_ != nullptr) {
+      parser->connectionsList_->Pop(parser);
+      parser->connectionsList_->PopActive(parser);
+    }
+  }
 
   void Save() {
     url_.Save();
@@ -1221,6 +1225,7 @@ void InitializeHttpParser(Local<Object> target,
   t->Inherit(AsyncWrap::GetConstructorTemplate(env));
   env->SetProtoMethod(t, "close", Parser::Close);
   env->SetProtoMethod(t, "free", Parser::Free);
+  env->SetProtoMethod(t, "remove", Parser::Remove);
   env->SetProtoMethod(t, "execute", Parser::Execute);
   env->SetProtoMethod(t, "finish", Parser::Finish);
   env->SetProtoMethod(t, "initialize", Parser::Initialize);
diff --git a/test/parallel/test-http-server-connection-list-when-close.js b/test/parallel/test-http-server-connection-list-when-close.js
new file mode 100644
index 00000000000000..94fcd1b0e8036f
--- /dev/null
+++ b/test/parallel/test-http-server-connection-list-when-close.js
@@ -0,0 +1,47 @@
+'use strict';
+
+const common = require('../common');
+const http = require('http');
+
+function request(server) {
+  http.get({
+    port: server.address().port,
+    path: '/',
+  }, (res) => {
+    res.resume();
+  });
+}
+
+{
+  const server = http.createServer((req, res) => {
+    //  Hack to not remove parser out of server.connectionList
+    //  See `freeParser` in _http_common.js
+    req.socket.parser.free = common.mustCall();
+    req.socket.on('close', common.mustCall(() => {
+      server.close();
+    }));
+    res.end('ok');
+  }).listen(0, common.mustCall(() => {
+    request(server);
+  }));
+}
+
+{
+  const server = http.createServer((req, res) => {
+    // See `freeParser` in _http_common.js
+    const { parser } = req.socket;
+    parser.free = common.mustCall(() => {
+      setImmediate(common.mustCall(() => {
+        parser.close();
+      }));
+    });
+    req.socket.on('close', common.mustCall(() => {
+      setImmediate(common.mustCall(() => {
+        server.close();
+      }));
+    }));
+    res.end('ok');
+  }).listen(0, common.mustCall(() => {
+    request(server);
+  }));
+}