From 40a2a8edc0943b7eb4b33b515d80803d2ef93e68 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 24 Aug 2019 20:42:11 +0200
Subject: [PATCH] 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
---
 lib/_http_server.js                           | 12 ++++++
 .../test-http-server-keepalive-end.js         | 15 +++++--
 .../test-http-server-keepalive-req-gc.js      | 42 +++++++++++++++++++
 3 files changed, 66 insertions(+), 3 deletions(-)
 create mode 100644 test/parallel/test-http-server-keepalive-req-gc.js

diff --git a/lib/_http_server.js b/lib/_http_server.js
index 862b7c970bda8e..6e285b3966795c 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -611,6 +611,17 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
   }
 }
 
+function clearIncoming(parser, req) {
+  // Reset the .incoming property so that the request object can be gc'ed.
+  if (parser && parser.incoming === req) {
+    if (req.complete) {
+      parser.incoming = null;
+    } else {
+      req.on('end', clearIncoming.bind(null, parser, req));
+    }
+  }
+}
+
 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 +629,7 @@ function resOnFinish(req, res, socket, state, server) {
   assert(state.incoming.length === 0 || state.incoming[0] === req);
 
   state.incoming.shift();
+  clearIncoming(socket.parser, 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..da701df02c11e1
--- /dev/null
+++ b/test/parallel/test-http-server-keepalive-req-gc.js
@@ -0,0 +1,42 @@
+// Flags: --expose-gc
+'use strict';
+const common = require('../common');
+const onGC = require('../common/ongc');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+// 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();
+}));