Skip to content

Commit 001881f

Browse files
davedoesdevtargos
authored andcommitted
http2: set nghttp2_option_set_no_closed_streams
PR-URL: #23134 Fixes: #23116 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 44db98a commit 001881f

File tree

2 files changed

+57
-0
lines changed

2 files changed

+57
-0
lines changed

src/node_http2.cc

+4
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ Http2Scope::~Http2Scope() {
9898
Http2Options::Http2Options(Environment* env, nghttp2_session_type type) {
9999
nghttp2_option_new(&options_);
100100

101+
// Make sure closed connections aren't kept around, taking up memory.
102+
// Note that this breaks the priority tree, which we don't use.
103+
nghttp2_option_set_no_closed_streams(options_, 1);
104+
101105
// We manually handle flow control within a session in order to
102106
// implement backpressure -- that is, we only send WINDOW_UPDATE
103107
// frames to the remote peer as data is actually consumed by user
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto) {
4+
common.skip('missing crypto');
5+
}
6+
7+
// Issue #23116
8+
// nghttp2 keeps closed stream structures around in memory (couple of hundred
9+
// bytes each) until a session is closed. It does this to maintain the priority
10+
// tree. However, it limits the number of requests that can be made in a
11+
// session before our memory tracking (correctly) kicks in.
12+
// The fix is to tell nghttp2 to forget about closed streams. We don't make use
13+
// of priority anyway.
14+
// Without the fix, this test fails at ~40k requests with an exception:
15+
// Error [ERR_HTTP2_STREAM_ERROR]: Stream closed with error code
16+
// NGHTTP2_ENHANCE_YOUR_CALM
17+
18+
const http2 = require('http2');
19+
const assert = require('assert');
20+
21+
const server = http2.createServer({ maxSessionMemory: 1 });
22+
23+
server.on('session', function(session) {
24+
session.on('stream', function(stream) {
25+
stream.on('end', common.mustCall(function() {
26+
this.respond({
27+
':status': 200
28+
}, {
29+
endStream: true
30+
});
31+
}));
32+
stream.resume();
33+
});
34+
});
35+
36+
server.listen(0, function() {
37+
const client = http2.connect(`http://localhost:${server.address().port}`);
38+
39+
function next(i) {
40+
if (i === 10000) {
41+
client.close();
42+
return server.close();
43+
}
44+
const stream = client.request({ ':method': 'POST' });
45+
stream.on('response', common.mustCall(function(headers) {
46+
assert.strictEqual(headers[':status'], 200);
47+
this.on('close', common.mustCall(() => next(i + 1)));
48+
}));
49+
stream.end();
50+
}
51+
52+
next(0);
53+
});

0 commit comments

Comments
 (0)