http2: strictly limit number on concurrent streams

Strictly limit the number of concurrent streams based on the
current setting of the MAX_CONCURRENT_STREAMS setting

PR-URL: https://github.com/nodejs/node/pull/16766
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
This commit is contained in:
James M Snell 2017-11-21 10:52:33 -08:00
parent d179ccac92
commit 653e894883
3 changed files with 80 additions and 1 deletions

View File

@ -655,6 +655,16 @@ inline Http2Stream* Http2Session::FindStream(int32_t id) {
return s != streams_.end() ? s->second : nullptr;
}
inline bool Http2Session::CanAddStream() {
uint32_t maxConcurrentStreams =
nghttp2_session_get_local_settings(
session_, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
size_t maxSize =
std::min(streams_.max_size(), static_cast<size_t>(maxConcurrentStreams));
// We can add a new stream so long as we are less than the current
// maximum on concurrent streams
return streams_.size() < maxSize;
}
inline void Http2Session::AddStream(Http2Stream* stream) {
CHECK_GE(++statistics_.stream_count, 0);
@ -765,7 +775,14 @@ inline int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
Http2Stream* stream = session->FindStream(id);
if (stream == nullptr) {
new Http2Stream(session, id, frame->headers.cat);
if (session->CanAddStream()) {
new Http2Stream(session, id, frame->headers.cat);
} else {
// Too many concurrent streams being opened
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
NGHTTP2_ENHANCE_YOUR_CALM);
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
} else {
// If the stream has already been destroyed, ignore.
if (stream->IsDestroyed())

View File

@ -835,6 +835,8 @@ class Http2Session : public AsyncWrap {
// Returns pointer to the stream, or nullptr if stream does not exist
inline Http2Stream* FindStream(int32_t id);
inline bool CanAddStream();
// Adds a stream instance to this session
inline void AddStream(Http2Stream* stream);

View File

@ -0,0 +1,60 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const Countdown = require('../common/countdown');
const http2 = require('http2');
const assert = require('assert');
// Test that the maxConcurrentStreams setting is strictly enforced
const server = http2.createServer({ settings: { maxConcurrentStreams: 1 } });
let c = 0;
server.on('stream', common.mustCall((stream) => {
// Because we only allow one open stream at a time,
// c should never be greater than 1.
assert.strictEqual(++c, 1);
stream.respond();
// Force some asynchronos stuff.
setImmediate(() => {
stream.end('ok');
assert.strictEqual(--c, 0);
});
}, 3));
server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const countdown = new Countdown(3, common.mustCall(() => {
server.close();
client.destroy();
}));
client.on('remoteSettings', common.mustCall(() => {
assert.strictEqual(client.remoteSettings.maxConcurrentStreams, 1);
{
const req = client.request();
req.resume();
req.on('close', () => {
countdown.dec();
setImmediate(() => {
const req = client.request();
req.resume();
req.on('close', () => countdown.dec());
});
});
}
{
const req = client.request();
req.resume();
req.on('close', () => countdown.dec());
}
}));
}));