From 001881f33ef73558cbca3f678ace011fc862c0dc Mon Sep 17 00:00:00 2001
From: David Halls <dahalls@gmail.com>
Date: Thu, 27 Sep 2018 22:28:50 +0100
Subject: [PATCH] http2: set nghttp2_option_set_no_closed_streams

PR-URL: https://github.com/nodejs/node/pull/23134
Fixes: https://github.com/nodejs/node/issues/23116
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/node_http2.cc                             |  4 ++
 .../test-http2-forget-closed-streams.js       | 53 +++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 test/parallel/test-http2-forget-closed-streams.js

diff --git a/src/node_http2.cc b/src/node_http2.cc
index d1c2362613a57a..1f649710a74a78 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -98,6 +98,10 @@ Http2Scope::~Http2Scope() {
 Http2Options::Http2Options(Environment* env, nghttp2_session_type type) {
   nghttp2_option_new(&options_);
 
+  // Make sure closed connections aren't kept around, taking up memory.
+  // Note that this breaks the priority tree, which we don't use.
+  nghttp2_option_set_no_closed_streams(options_, 1);
+
   // We manually handle flow control within a session in order to
   // implement backpressure -- that is, we only send WINDOW_UPDATE
   // frames to the remote peer as data is actually consumed by user
diff --git a/test/parallel/test-http2-forget-closed-streams.js b/test/parallel/test-http2-forget-closed-streams.js
new file mode 100644
index 00000000000000..c0b3bcd819140c
--- /dev/null
+++ b/test/parallel/test-http2-forget-closed-streams.js
@@ -0,0 +1,53 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto) {
+  common.skip('missing crypto');
+}
+
+// Issue #23116
+// nghttp2 keeps closed stream structures around in memory (couple of hundred
+// bytes each) until a session is closed. It does this to maintain the priority
+// tree. However, it limits the number of requests that can be made in a
+// session before our memory tracking (correctly) kicks in.
+// The fix is to tell nghttp2 to forget about closed streams. We don't make use
+// of priority anyway.
+// Without the fix, this test fails at ~40k requests with an exception:
+// Error [ERR_HTTP2_STREAM_ERROR]: Stream closed with error code
+// NGHTTP2_ENHANCE_YOUR_CALM
+
+const http2 = require('http2');
+const assert = require('assert');
+
+const server = http2.createServer({ maxSessionMemory: 1 });
+
+server.on('session', function(session) {
+  session.on('stream', function(stream) {
+    stream.on('end', common.mustCall(function() {
+      this.respond({
+        ':status': 200
+      }, {
+        endStream: true
+      });
+    }));
+    stream.resume();
+  });
+});
+
+server.listen(0, function() {
+  const client = http2.connect(`http://localhost:${server.address().port}`);
+
+  function next(i) {
+    if (i === 10000) {
+      client.close();
+      return server.close();
+    }
+    const stream = client.request({ ':method': 'POST' });
+    stream.on('response', common.mustCall(function(headers) {
+      assert.strictEqual(headers[':status'], 200);
+      this.on('close', common.mustCall(() => next(i + 1)));
+    }));
+    stream.end();
+  }
+
+  next(0);
+});