Skip to content

Commit

Permalink
http: fix http server connection list when close
Browse files Browse the repository at this point in the history
PR-URL: #43949
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
  • Loading branch information
theanarkh authored and danielleadams committed Jul 26, 2022
1 parent 61cd11a commit 25ec71d
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 5 deletions.
1 change: 1 addition & 0 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
15 changes: 10 additions & 5 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
47 changes: 47 additions & 0 deletions test/parallel/test-http-server-connection-list-when-close.js
Original file line number Diff line number Diff line change
@@ -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);
}));
}

0 comments on commit 25ec71d

Please sign in to comment.