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>
This commit is contained in:
parent
2f36cb4125
commit
e8121d52db
@ -98,6 +98,10 @@ Http2Scope::~Http2Scope() {
|
|||||||
Http2Options::Http2Options(Environment* env, nghttp2_session_type type) {
|
Http2Options::Http2Options(Environment* env, nghttp2_session_type type) {
|
||||||
nghttp2_option_new(&options_);
|
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
|
// We manually handle flow control within a session in order to
|
||||||
// implement backpressure -- that is, we only send WINDOW_UPDATE
|
// implement backpressure -- that is, we only send WINDOW_UPDATE
|
||||||
// frames to the remote peer as data is actually consumed by user
|
// frames to the remote peer as data is actually consumed by user
|
||||||
|
53
test/parallel/test-http2-forget-closed-streams.js
Normal file
53
test/parallel/test-http2-forget-closed-streams.js
Normal file
@ -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);
|
||||||
|
});
|
Loading…
x
Reference in New Issue
Block a user