Skip to content

Commit 1a09b4d

Browse files
leidegreaddaleax
authored andcommitted
http: fixes memory retention issue with FreeList and HTTPParser
Fixes: #29394 Refs: #33167 (comment) PR-URL: #33190 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>
1 parent c9bd1a7 commit 1a09b4d

File tree

2 files changed

+44
-2
lines changed

2 files changed

+44
-2
lines changed

lib/_http_common.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,10 @@ const parsers = new FreeList('parsers', 1000, function parsersCb() {
164164

165165
cleanParser(parser);
166166

167-
parser.onIncoming = null;
168167
parser[kOnHeaders] = parserOnHeaders;
169168
parser[kOnHeadersComplete] = parserOnHeadersComplete;
170169
parser[kOnBody] = parserOnBody;
171170
parser[kOnMessageComplete] = parserOnMessageComplete;
172-
parser[kOnTimeout] = null;
173171

174172
return parser;
175173
});
@@ -235,7 +233,9 @@ function cleanParser(parser) {
235233
parser.outgoing = null;
236234
parser.maxHeaderPairs = MAX_HEADER_PAIRS;
237235
parser[kOnExecute] = null;
236+
parser[kOnTimeout] = null;
238237
parser._consumed = false;
238+
parser.onIncoming = null;
239239
}
240240

241241
function prepareError(err, parser, rawPacket) {
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const http = require('http');
6+
const { HTTPParser } = require('_http_common');
7+
8+
// Test that the `HTTPParser` instance is cleaned up before being returned to
9+
// the pool to avoid memory retention issues.
10+
11+
const kOnTimeout = HTTPParser.kOnTimeout | 0;
12+
const server = http.createServer();
13+
14+
server.on('request', common.mustCall((request, response) => {
15+
const parser = request.socket.parser;
16+
17+
assert.strictEqual(typeof parser[kOnTimeout], 'function');
18+
19+
request.socket.on('close', common.mustCall(() => {
20+
assert.strictEqual(parser[kOnTimeout], null);
21+
}));
22+
23+
response.end();
24+
server.close();
25+
}));
26+
27+
server.listen(common.mustCall(() => {
28+
const request = http.get({ port: server.address().port });
29+
let parser;
30+
31+
request.on('socket', common.mustCall(() => {
32+
parser = request.parser;
33+
assert.strictEqual(typeof parser.onIncoming, 'function');
34+
}));
35+
36+
request.on('response', common.mustCall((response) => {
37+
response.resume();
38+
response.on('end', common.mustCall(() => {
39+
assert.strictEqual(parser.onIncoming, null);
40+
}));
41+
}));
42+
}));

0 commit comments

Comments
 (0)