From cc02c73e5395e5c3dcbc9c5c90829b00dd1eb11d Mon Sep 17 00:00:00 2001 From: John Leidegren Date: Fri, 1 May 2020 08:14:25 +0200 Subject: [PATCH] http: fixes memory retention issue with FreeList and HTTPParser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/nodejs/node/issues/29394 Refs: https://github.com/nodejs/node/pull/33167#issuecomment-622102450 PR-URL: https://github.com/nodejs/node/pull/33190 Reviewed-By: Robert Nagy Reviewed-By: Luigi Pinca Reviewed-By: Juan José Arboleda Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott Reviewed-By: Gerhard Stöbich --- lib/_http_common.js | 4 +- .../test-http-parser-memory-retention.js | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-http-parser-memory-retention.js diff --git a/lib/_http_common.js b/lib/_http_common.js index eec965d7fcf8f1..2c836221f731e1 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -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; }); @@ -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) { diff --git a/test/parallel/test-http-parser-memory-retention.js b/test/parallel/test-http-parser-memory-retention.js new file mode 100644 index 00000000000000..3a6d34221e0bda --- /dev/null +++ b/test/parallel/test-http-parser-memory-retention.js @@ -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); + })); + })); +}));