http: reset parser.incoming when server request is finished
This resolves a memory leak for keep-alive connections and does not regress in the way that 779a05d5d1bfe2eeb05386f did by waiting for the incoming request to be finished before releasing the `parser.incoming` object. Refs: https://github.com/nodejs/node/pull/28646 Refs: https://github.com/nodejs/node/pull/29263 Fixes: https://github.com/nodejs/node/issues/9668 PR-URL: https://github.com/nodejs/node/pull/29297 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
parent
698a29420f
commit
6ab28486c3
@ -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
|
||||
|
@ -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 = [
|
||||
|
47
test/parallel/test-http-server-keepalive-req-gc.js
Normal file
47
test/parallel/test-http-server-keepalive-req-gc.js
Normal file
@ -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();
|
||||
}));
|
Loading…
x
Reference in New Issue
Block a user