A follow-up to a recent fix in QHttpNetworkConnectionChannel

While working with HTTP/2, we are not re-sending failed requests.
In case we receive a GOAWAY frame, we properly handle it by
processing some active streams if possible, and aborting streams
that will not proceed further with ContentResendError. But it's
possible that some server failed to send us GOAWAY (for example,
it died) or closed the connection not finishing the streams that
were still active and valid (ID <= value from GOAWAY frame).
Now that we will not re-connect, there is no reason to be quiet
about us not progressing - emit RemoteHostClosedError on any
remaining active stream/request we cannot process further.

Fixes: QTBUG-77852
Change-Id: I4cd68a1c8c103b1fbe36c20a1cc406ab2e20dd12
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
bb10
Timur Pocheptsov 2019-09-04 15:00:31 +02:00
parent d49daa3f27
commit 543769666f
3 changed files with 39 additions and 1 deletions

View File

@ -58,6 +58,8 @@
#include <QtNetwork/qnetworkproxy.h>
#endif
#include <qcoreapplication.h>
#include <algorithm>
#include <vector>
@ -195,6 +197,29 @@ QHttp2ProtocolHandler::QHttp2ProtocolHandler(QHttpNetworkConnectionChannel *chan
}
}
void QHttp2ProtocolHandler::handleConnectionClosure()
{
// The channel has just received RemoteHostClosedError and since it will
// not try (for HTTP/2) to re-connect, it's time to finish all replies
// with error.
// Maybe we still have some data to read and can successfully finish
// a stream/request?
_q_receiveReply();
// Finish all still active streams. If we previously had GOAWAY frame,
// we probably already closed some (or all) streams with ContentReSend
// error, but for those still active, not having any data to finish,
// we now report RemoteHostClosedError.
const auto errorString = QCoreApplication::translate("QHttp", "Connection closed");
for (auto it = activeStreams.begin(), eIt = activeStreams.end(); it != eIt; ++it)
finishStreamWithError(it.value(), QNetworkReply::RemoteHostClosedError, errorString);
// Make sure we'll never try to read anything later:
activeStreams.clear();
goingAway = true;
}
void QHttp2ProtocolHandler::_q_uploadDataReadyRead()
{
if (!sender()) // QueuedConnection, firing after sender (byte device) was deleted.

View File

@ -92,6 +92,8 @@ public:
QHttp2ProtocolHandler &operator = (const QHttp2ProtocolHandler &rhs) = delete;
QHttp2ProtocolHandler &operator = (QHttp2ProtocolHandler &&rhs) = delete;
Q_INVOKABLE void handleConnectionClosure();
private slots:
void _q_uploadDataReadyRead();
void _q_replyDestroyed(QObject* reply);

View File

@ -977,7 +977,18 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
if (!reply && state == QHttpNetworkConnectionChannel::IdleState) {
// Not actually an error, it is normal for Keep-Alive connections to close after some time if no request
// is sent on them. No need to error the other replies below. Just bail out here.
// The _q_disconnected will handle the possibly pipelined replies
// The _q_disconnected will handle the possibly pipelined replies. HTTP/2 is special for now,
// we do not resend, but must report errors if any request is in progress (note, while
// not in its sendRequest(), protocol handler switches the channel to IdleState, thus
// this check is under this condition in 'if'):
if (protocolHandler.data()) {
if (connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2Direct
|| connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2) {
auto h2Handler = static_cast<QHttp2ProtocolHandler *>(protocolHandler.data());
h2Handler->handleConnectionClosure();
protocolHandler.reset();
}
}
return;
} else if (state != QHttpNetworkConnectionChannel::IdleState && state != QHttpNetworkConnectionChannel::ReadingState) {
// Try to reconnect/resend before sending an error.