diff --git a/lib/_http_server.js b/lib/_http_server.js index 862b7c970bda8e..6dc29c855e414c 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -611,6 +611,19 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { } } +function clearIncoming(req) { + req = req || this; + const parser = req.socket && req.socket.parser; + // Reset the .incoming property so that the request object can be gc'ed. + if (parser && parser.incoming === req) { + if (req.readableEnded) { + parser.incoming = null; + } else { + req.on('end', clearIncoming); + } + } +} + function resOnFinish(req, res, socket, state, server) { // Usually the first incoming element should be our request. it may // be that in the case abortIncoming() was called that the incoming @@ -618,6 +631,7 @@ function resOnFinish(req, res, socket, state, server) { assert(state.incoming.length === 0 || state.incoming[0] === req); state.incoming.shift(); + clearIncoming(req); // If the user never called req.read(), and didn't pipe() or // .resume() or .on('data'), then we call req._dump() so that the diff --git a/test/parallel/test-http-server-keepalive-end.js b/test/parallel/test-http-server-keepalive-end.js index 1e3a44d368a020..8ebdecb97c3d8c 100644 --- a/test/parallel/test-http-server-keepalive-end.js +++ b/test/parallel/test-http-server-keepalive-end.js @@ -1,18 +1,27 @@ 'use strict'; - const common = require('../common'); +const assert = require('assert'); const { createServer } = require('http'); const { connect } = require('net'); +// Make sure that for HTTP keepalive requests, the .on('end') event is emitted +// on the incoming request object, and that the parser instance does not hold +// on to that request object afterwards. + const server = createServer(common.mustCall((req, res) => { - req.on('end', common.mustCall()); + req.on('end', common.mustCall(() => { + const parser = req.socket.parser; + assert.strictEqual(parser.incoming, req); + process.nextTick(() => { + assert.strictEqual(parser.incoming, null); + }); + })); res.end('hello world'); })); server.unref(); server.listen(0, common.mustCall(() => { - const client = connect(server.address().port); const req = [ diff --git a/test/parallel/test-http-server-keepalive-req-gc.js b/test/parallel/test-http-server-keepalive-req-gc.js new file mode 100644 index 00000000000000..aa4bf1a3de9c83 --- /dev/null +++ b/test/parallel/test-http-server-keepalive-req-gc.js @@ -0,0 +1,47 @@ +// Flags: --expose-gc +'use strict'; +const common = require('../common'); +const onGC = require('../common/ongc'); +const { createServer } = require('http'); +const { connect } = require('net'); + +if (common.isWindows) { + // TODO(addaleax): Investigate why and remove the skip. + common.skip('This test is flaky on Windows.'); +} + +// Make sure that for HTTP keepalive requests, the req object can be +// garbage collected once the request is finished. +// Refs: https://github.com/nodejs/node/issues/9668 + +let client; +const server = createServer(common.mustCall((req, res) => { + onGC(req, { ongc: common.mustCall() }); + req.resume(); + req.on('end', common.mustCall(() => { + setImmediate(() => { + client.end(); + global.gc(); + }); + })); + res.end('hello world'); +})); + +server.unref(); + +server.listen(0, common.mustCall(() => { + client = connect(server.address().port); + + const req = [ + 'POST / HTTP/1.1', + `Host: localhost:${server.address().port}`, + 'Connection: keep-alive', + 'Content-Length: 11', + '', + 'hello world', + '' + ].join('\r\n'); + + client.write(req); + client.unref(); +}));