Http/2: fix active streams counting

We were looking at all active streams, but that also includes promised
streams.

By the RFC the limitation that our peer specifies only applies to the
number of streams _we_ create, not the total amount of active streams.

More importantly, for the qhttp2protocolhandler it could mean that we
could end up having a few promised streams push the active stream count
over the limit, which could lead us to start more streams than intended
(then bounded by the number of queued requests).

The worst case in this scenario is that a **non-compliant** server
doesn't track how many connections we open and the user has queued
a ton of requests, so we open a ton of streams.

But on the upside: server-push is not widely used.

Change-Id: I2a533472eb9127fd176bb99e9db0518f05c3fffe
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by:  Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 5ed736b053eb9d04ecd1a6f2f375cce7fcefe4e6)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Mårten Nordheim 2024-02-07 18:56:19 +01:00 committed by Qt Cherry-pick Bot
parent 933d695ecb
commit 305deaca22
3 changed files with 34 additions and 8 deletions

View File

@ -552,7 +552,7 @@ QHttp2Connection::createStreamInternal()
if (m_goingAway)
return { QHttp2Connection::CreateStreamError::ReceivedGOAWAY };
const quint32 streamID = m_nextStreamID;
if (size_t(m_maxConcurrentStreams) <= size_t(numActiveStreams()))
if (size_t(m_maxConcurrentStreams) <= size_t(numActiveLocalStreams()))
return { QHttp2Connection::CreateStreamError::MaxConcurrentStreamsReached };
m_nextStreamID += 2;
return { createStreamInternal_impl(streamID) };
@ -570,12 +570,32 @@ QHttp2Stream *QHttp2Connection::createStreamInternal_impl(quint32 streamID)
return stream;
}
qsizetype QHttp2Connection::numActiveStreams() const noexcept
qsizetype QHttp2Connection::numActiveStreamsImpl(quint32 mask) const noexcept
{
return std::count_if(m_streams.cbegin(), m_streams.cend(),
[](const QPointer<QHttp2Stream> &stream) {
return stream && stream->state() == QHttp2Stream::State::Open;
});
const auto shouldCount = [mask](const QPointer<QHttp2Stream> &stream) -> bool {
return stream && (stream->streamID() & 1) == mask;
};
return std::count_if(m_streams.cbegin(), m_streams.cend(), shouldCount);
}
/*!
\internal
The number of streams the remote peer has started that are still active.
*/
qsizetype QHttp2Connection::numActiveRemoteStreams() const noexcept
{
const quint32 RemoteMask = m_connectionType == Type::Client ? 0 : 1;
return numActiveStreamsImpl(RemoteMask);
}
/*!
\internal
The number of streams we have started that are still active.
*/
qsizetype QHttp2Connection::numActiveLocalStreams() const noexcept
{
const quint32 LocalMask = m_connectionType == Type::Client ? 1 : 0;
return numActiveStreamsImpl(LocalMask);
}
QHttp2Stream *QHttp2Connection::getStream(quint32 streamID) const

View File

@ -20,6 +20,7 @@
#include <QtCore/qobject.h>
#include <QtCore/qhash.h>
#include <QtCore/qvarlengtharray.h>
#include <QtCore/qxpfunctional.h>
#include <QtNetwork/qhttp2configuration.h>
#include <QtNetwork/qtcpsocket.h>
@ -253,7 +254,9 @@ private:
const char *message); // Connection failed to be established?
void setH2Configuration(QHttp2Configuration config);
void closeSession();
qsizetype numActiveStreams() const noexcept;
qsizetype numActiveStreamsImpl(quint32 mask) const noexcept;
qsizetype numActiveRemoteStreams() const noexcept;
qsizetype numActiveLocalStreams() const noexcept;
bool sendClientPreface();
bool sendSETTINGS();

View File

@ -325,7 +325,10 @@ bool QHttp2ProtocolHandler::sendRequest()
initReplyFromPushPromise(message, key);
}
const qint64 streamsToUse = qBound(0, qint64(maxConcurrentStreams) - activeStreams.size(),
const auto isClientSide = [](const auto &pair) -> bool { return (pair.first & 1) == 1; };
const auto activeClientSideStreams = std::count_if(
activeStreams.constKeyValueBegin(), activeStreams.constKeyValueEnd(), isClientSide);
const qint64 streamsToUse = qBound(0, qint64(maxConcurrentStreams) - activeClientSideStreams,
requests.size());
auto it = requests.begin();
for (qint64 i = 0; i < streamsToUse; ++i) {