http: fix http server connection list when close

PR-URL: #43949 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Feng Yu <F3n67u@outlook.com>
nodejs · Jul 26, 2022 · 25ec71db63d6c46a5397a75a9e7ed97e3b5646d5 · 25ec71d
1 parent 61cd11a
commit 25ec71db63d6c46a5397a75a9e7ed97e3b5646d5
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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);
+  }));
+}