Skip to content

Commit

Permalink
http: fixes memory retention issue with FreeList and HTTPParser
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
leidegre authored and codebytere committed May 7, 2020
1 parent c5a38fe commit cc02c73
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,10 @@ const parsers = new FreeList('parsers', 1000, function parsersCb() {

cleanParser(parser);

parser.onIncoming = null;
parser[kOnHeaders] = parserOnHeaders;
parser[kOnHeadersComplete] = parserOnHeadersComplete;
parser[kOnBody] = parserOnBody;
parser[kOnMessageComplete] = parserOnMessageComplete;
parser[kOnTimeout] = null;

return parser;
});
Expand Down Expand Up @@ -232,7 +230,9 @@ function cleanParser(parser) {
parser.outgoing = null;
parser.maxHeaderPairs = MAX_HEADER_PAIRS;
parser[kOnExecute] = null;
parser[kOnTimeout] = null;
parser._consumed = false;
parser.onIncoming = null;
}

function prepareError(err, parser, rawPacket) {
Expand Down
42 changes: 42 additions & 0 deletions test/parallel/test-http-parser-memory-retention.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');
const { HTTPParser } = require('_http_common');

// Test that the `HTTPParser` instance is cleaned up before being returned to
// the pool to avoid memory retention issues.

const kOnTimeout = HTTPParser.kOnTimeout | 0;
const server = http.createServer();

server.on('request', common.mustCall((request, response) => {
const parser = request.socket.parser;

assert.strictEqual(typeof parser[kOnTimeout], 'function');

request.socket.on('close', common.mustCall(() => {
assert.strictEqual(parser[kOnTimeout], null);
}));

response.end();
server.close();
}));

server.listen(common.mustCall(() => {
const request = http.get({ port: server.address().port });
let parser;

request.on('socket', common.mustCall(() => {
parser = request.parser;
assert.strictEqual(typeof parser.onIncoming, 'function');
}));

request.on('response', common.mustCall((response) => {
response.resume();
response.on('end', common.mustCall(() => {
assert.strictEqual(parser.onIncoming, null);
}));
}));
}));

0 comments on commit cc02c73

Please sign in to comment.