File qt5-correctly-handle-connection-closed.patch of Package mingw32-libqt5-qtbase

Overview Repositories Revisions Requests Users Attributes Meta

File qt5-correctly-handle-connection-closed.patch of Package mingw32-libqt5-qtbase (Revision 6bdcabd9eab97cc765431140b75ef254)

Currently displaying revision 6bdcabd9eab97cc765431140b75ef254 , Show latest

commit 330c4452e06f5af4c650fb3d5f70d0b815b68284
Author: Markus Goetz <markus@woboq.com>
Date:   Fri Jun 19 15:35:34 2015 +0200

QNAM: Fix up previous corruption patch

diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp
index 365ce55..543c70e 100644
--- a/src/network/access/qhttpnetworkconnection.cpp
+++ b/src/network/access/qhttpnetworkconnection.cpp
@@ -917,7 +917,6 @@ void QHttpNetworkConnectionPrivate::_q_startNextRequest()
     for (int i = 0; i < channelCount; ++i) {
         if (channels[i].resendCurrent && (channels[i].state != QHttpNetworkConnectionChannel::ClosingState)) {
             channels[i].resendCurrent = false;
-            channels[i].state = QHttpNetworkConnectionChannel::IdleState;
 
             // if this is not possible, error will be emitted and connection terminated
             if (!channels[i].resetUploadData())
diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
index 19c92e7..b189c08 100644
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -58,6 +58,11 @@ QT_BEGIN_NAMESPACE
 
 // TODO: Put channel specific stuff here so it does not polute qhttpnetworkconnection.cpp
 
+// Because in-flight when sending a request, the server might close our connection (because the persistent HTTP
+// connection times out)
+// We use 3 because we can get a _q_error 3 times depending on the timing:
+static const int reconnectAttemptsDefault = 3;
+
 QHttpNetworkConnectionChannel::QHttpNetworkConnectionChannel()
     : socket(0)
     , ssl(false)
@@ -69,7 +74,7 @@ QHttpNetworkConnectionChannel::QHttpNetworkConnectionChannel()
     , resendCurrent(false)
     , lastStatus(0)
     , pendingEncrypt(false)
-    , reconnectAttempts(2)
+    , reconnectAttempts(reconnectAttemptsDefault)
     , authMethod(QAuthenticatorPrivate::None)
     , proxyAuthMethod(QAuthenticatorPrivate::None)
     , authenticationCredentialsSent(false)
@@ -106,19 +111,18 @@ void QHttpNetworkConnectionChannel::init()
     socket->setProxy(QNetworkProxy::NoProxy);
 #endif
 
-    // We want all signals (except the interactive ones) be connected as QueuedConnection
-    // because else we're falling into cases where we recurse back into the socket code
-    // and mess up the state. Always going to the event loop (and expecting that when reading/writing)
-    // is safer.
+    // After some back and forth in all the last years, this is now a DirectConnection because otherwise
+    // the state inside the *Socket classes gets messed up, also in conjunction with the socket notifiers
+    // which behave slightly differently on Windows vs Linux
     QObject::connect(socket, SIGNAL(bytesWritten(qint64)),
                      this, SLOT(_q_bytesWritten(qint64)),
-                     Qt::QueuedConnection);
+                     Qt::DirectConnection);
     QObject::connect(socket, SIGNAL(connected()),
                      this, SLOT(_q_connected()),
-                     Qt::QueuedConnection);
+                     Qt::DirectConnection);
     QObject::connect(socket, SIGNAL(readyRead()),
                      this, SLOT(_q_readyRead()),
-                     Qt::QueuedConnection);
+                     Qt::DirectConnection);
 
     // The disconnected() and error() signals may already come
     // while calling connectToHost().
@@ -129,10 +133,10 @@ void QHttpNetworkConnectionChannel::init()
     qRegisterMetaType<QAbstractSocket::SocketError>();
     QObject::connect(socket, SIGNAL(disconnected()),
                      this, SLOT(_q_disconnected()),
-                     Qt::QueuedConnection);
+                     Qt::DirectConnection);
     QObject::connect(socket, SIGNAL(error(QAbstractSocket::SocketError)),
                      this, SLOT(_q_error(QAbstractSocket::SocketError)),
-                     Qt::QueuedConnection);
+                     Qt::DirectConnection);
 
 
 #ifndef QT_NO_NETWORKPROXY
@@ -147,13 +151,13 @@ void QHttpNetworkConnectionChannel::init()
         // won't be a sslSocket if encrypt is false
         QObject::connect(sslSocket, SIGNAL(encrypted()),
                          this, SLOT(_q_encrypted()),
-                         Qt::QueuedConnection);
+                         Qt::DirectConnection);
         QObject::connect(sslSocket, SIGNAL(sslErrors(QList<QSslError>)),
                          this, SLOT(_q_sslErrors(QList<QSslError>)),
                          Qt::DirectConnection);
         QObject::connect(sslSocket, SIGNAL(encryptedBytesWritten(qint64)),
                          this, SLOT(_q_encryptedBytesWritten(qint64)),
-                         Qt::QueuedConnection);
+                         Qt::DirectConnection);
 
         if (ignoreAllSslErrors)
             sslSocket->ignoreSslErrors();
@@ -180,6 +184,7 @@ void QHttpNetworkConnectionChannel::init()
 
 void QHttpNetworkConnectionChannel::close()
 {
+    // qDebug() << Q_FUNC_INFO << socket << state << reply;
     if (!socket)
         state = QHttpNetworkConnectionChannel::IdleState;
     else if (socket->state() == QAbstractSocket::UnconnectedState)
@@ -221,6 +226,7 @@ void QHttpNetworkConnectionChannel::_q_readyRead()
 void QHttpNetworkConnectionChannel::handleUnexpectedEOF()
 {
     Q_ASSERT(reply);
+    // qDebug() << Q_FUNC_INFO << socket << reconnectAttempts << state;
     if (reconnectAttempts <= 0) {
         // too many errors reading/receiving/parsing the status, close the socket and emit error
         requeueCurrentlyPipelinedRequests();
@@ -398,7 +404,7 @@ void QHttpNetworkConnectionChannel::allDone()
 
     // reset the reconnection attempts after we receive a complete reply.
     // in case of failures, each channel will attempt two reconnects before emitting error.
-    reconnectAttempts = 2;
+    reconnectAttempts = reconnectAttemptsDefault;
 
     // now the channel can be seen as free/idle again, all signal emissions for the reply have been done
     if (state != QHttpNetworkConnectionChannel::ClosingState)
@@ -652,6 +658,15 @@ void QHttpNetworkConnectionChannel::closeAndResendCurrentRequest()
         QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
 }
 
+void QHttpNetworkConnectionChannel::resendCurrentRequest()
+{
+    requeueCurrentlyPipelinedRequests();
+    if (reply)
+        resendCurrent = true;
+    if (qobject_cast<QHttpNetworkConnection*>(connection))
+        QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
+}
+
 bool QHttpNetworkConnectionChannel::isSocketBusy() const
 {
     return (state & QHttpNetworkConnectionChannel::BusyState);
@@ -689,14 +704,15 @@ void QHttpNetworkConnectionChannel::_q_bytesWritten(qint64 bytes)
 
 void QHttpNetworkConnectionChannel::_q_disconnected()
 {
+    // qDebug() << Q_FUNC_INFO << (socket ? socket->state() : -1) << socket << reply << socket->bytesAvailable() << resendCurrent <<reconnectAttempts << state;
     if (state == QHttpNetworkConnectionChannel::ClosingState) {
         state = QHttpNetworkConnectionChannel::IdleState;
         QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
         return;
     }
 
-    // read the available data before closing
-    if (isSocketWaiting() || isSocketReading()) {
+    // read the available data before closing (also done in _q_error for other codepaths)
+    if ((isSocketWaiting() || isSocketReading()) && socket->bytesAvailable()) {
         if (reply) {
             state = QHttpNetworkConnectionChannel::ReadingState;
             _q_receiveReply();
@@ -708,7 +724,8 @@ void QHttpNetworkConnectionChannel::_q_disconnected()
     state = QHttpNetworkConnectionChannel::IdleState;
 
     requeueCurrentlyPipelinedRequests();
-    close();
+
+    pendingEncrypt = false;
 }
 
 
@@ -780,6 +797,8 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
 {
     if (!socket)
         return;
+    // qDebug() <<Q_FUNC_INFO << socketError << socket << reply << socket->bytesAvailable() << resendCurrent <<reconnectAttempts << state << socket->state();
+
     QNetworkReply::NetworkError errorCode = QNetworkReply::UnknownNetworkError;
 
     switch (socketError) {
@@ -790,11 +809,18 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
         errorCode = QNetworkReply::ConnectionRefusedError;
         break;
     case QAbstractSocket::RemoteHostClosedError:
-        // try to reconnect/resend before sending an error.
-        // while "Reading" the _q_disconnected() will handle this.
-        if (state != QHttpNetworkConnectionChannel::IdleState && state != QHttpNetworkConnectionChannel::ReadingState) {
+        // This error for SSL comes twice in a row, first from SSL layer ("The TLS/SSL connection has been closed") then from TCP layer.
+        // Depending on timing it can also come three times in a row (first time when we try to write into a closing QSslSocket).
+        // The reconnectAttempts handling catches the cases where we can re-send the request
+        if (!reply && state == QHttpNetworkConnectionChannel::IdleState) {
+            // Not actually an error, no need to error the other replies below. Just bail out here.
+            // The _q_disconnected will handle the possibly pipelined replies
+            return;
+        } else if (state != QHttpNetworkConnectionChannel::IdleState && state != QHttpNetworkConnectionChannel::ReadingState) {
+            // try to reconnect/resend before sending an error.
+            // while "Reading" the _q_disconnected() will handle this.
             if (reconnectAttempts-- > 0) {
-                closeAndResendCurrentRequest();
+                resendCurrentRequest();
                 return;
             } else {
                 errorCode = QNetworkReply::RemoteHostClosedError;
@@ -819,24 +845,15 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
                 // we can ignore the readbuffersize as the data is already
                 // in memory and we will not receive more data on the socket.
                 reply->setReadBufferSize(0);
+                reply->setDownstreamLimited(false);
                 _q_receiveReply();
-#ifndef QT_NO_SSL
-                if (ssl) {
-                    // QT_NO_OPENSSL. The QSslSocket can still have encrypted bytes in the plainsocket.
-                    // So we need to check this if the socket is a QSslSocket. When the socket is flushed
-                    // it will force a decrypt of the encrypted data in the plainsocket.
-                    QSslSocket *sslSocket = static_cast<QSslSocket*>(socket);
-                    qint64 beforeFlush = sslSocket->encryptedBytesAvailable();
-                    while (sslSocket->encryptedBytesAvailable()) {
-                        sslSocket->flush();
-                        _q_receiveReply();
-                        qint64 afterFlush = sslSocket->encryptedBytesAvailable();
-                        if (afterFlush == beforeFlush)
-                            break;
-                        beforeFlush = afterFlush;
-                    }
+                if (!reply) {
+                    // No more reply? Then it had been finished sucessfully.
+                    requeueCurrentlyPipelinedRequests();
+                    state = QHttpNetworkConnectionChannel::IdleState;
+                    QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
+                    return;
                 }
-#endif
             }
 
             errorCode = QNetworkReply::RemoteHostClosedError;
@@ -847,7 +864,7 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
     case QAbstractSocket::SocketTimeoutError:
         // try to reconnect/resend before sending an error.
         if (state == QHttpNetworkConnectionChannel::WritingState && (reconnectAttempts-- > 0)) {
-            closeAndResendCurrentRequest();
+            resendCurrentRequest();
             return;
         }
         errorCode = QNetworkReply::TimeoutError;
@@ -861,7 +878,7 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
     case QAbstractSocket::ProxyConnectionClosedError:
         // try to reconnect/resend before sending an error.
         if (reconnectAttempts-- > 0) {
-            closeAndResendCurrentRequest();
+            resendCurrentRequest();
             return;
         }
         errorCode = QNetworkReply::ProxyConnectionClosedError;
@@ -869,7 +886,7 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
     case QAbstractSocket::ProxyConnectionTimeoutError:
         // try to reconnect/resend before sending an error.
         if (reconnectAttempts-- > 0) {
-            closeAndResendCurrentRequest();
+            resendCurrentRequest();
             return;
         }
         errorCode = QNetworkReply::ProxyTimeoutError;
@@ -888,10 +905,14 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
         return;
 
     // emit error for all waiting replies
+    if (reply) qDebug() << "ERRORING CURRENT" << reply << reply->request().url();
     do {
         // Need to dequeu the request so that we can emit the error.
-        if (!reply)
+        if (!reply) {
             connection->d_func()->dequeueRequest(socket);
+            if (reply)
+                qDebug() << "ERRORING OTHER" << reply << reply->request().url();
+        }
 
         if (reply) {
             reply->d_func()->errorString = errorString;
@@ -917,8 +938,18 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
     // send the next request
     QMetaObject::invokeMethod(that, "_q_startNextRequest", Qt::QueuedConnection);
 
-    if (that) //signal emission triggered event loop
-        close();
+    if (that) {
+        //signal emission triggered event loop
+        if (!socket)
+            state = QHttpNetworkConnectionChannel::IdleState;
+        else if (socket->state() == QAbstractSocket::UnconnectedState)
+            state = QHttpNetworkConnectionChannel::IdleState;
+        else
+            state = QHttpNetworkConnectionChannel::ClosingState;
+
+        // pendingEncrypt must only be true in between connected and encrypted states
+        pendingEncrypt = false;
+    }
 }
 
 #ifndef QT_NO_NETWORKPROXY
@@ -942,7 +973,8 @@ void QHttpNetworkConnectionChannel::_q_proxyAuthenticationRequired(const QNetwor
 
 void QHttpNetworkConnectionChannel::_q_uploadDataReadyRead()
 {
-    sendRequest();
+    if (reply)
+        sendRequest();
 }
 
 #ifndef QT_NO_SSL
diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h
index 231fe11..a834b7d 100644
--- a/src/network/access/qhttpnetworkconnectionchannel_p.h
+++ b/src/network/access/qhttpnetworkconnectionchannel_p.h
@@ -169,6 +169,7 @@ public:
 
     void handleUnexpectedEOF();
     void closeAndResendCurrentRequest();
+    void resendCurrentRequest();
 
     bool isSocketBusy() const;
     bool isSocketWriting() const;
diff --git a/src/network/access/qhttpnetworkreply.cpp b/src/network/access/qhttpnetworkreply.cpp
index 55863a3..8b71bd8 100644
--- a/src/network/access/qhttpnetworkreply.cpp
+++ b/src/network/access/qhttpnetworkreply.cpp
@@ -191,7 +191,7 @@ QByteArray QHttpNetworkReply::readAny()
         return QByteArray();
 
     // we'll take the last buffer, so schedule another read from http
-    if (d->downstreamLimited && d->responseData.bufferCount() == 1)
+    if (d->downstreamLimited && d->responseData.bufferCount() == 1 && !isFinished())
         d->connection->d_func()->readMoreLater(this);
     return d->responseData.read();
 }
diff --git a/src/network/access/qhttpprotocolhandler.cpp b/src/network/access/qhttpprotocolhandler.cpp
index c9c871c..982d670 100644
--- a/src/network/access/qhttpprotocolhandler.cpp
+++ b/src/network/access/qhttpprotocolhandler.cpp
@@ -75,6 +75,7 @@ void QHttpProtocolHandler::_q_receiveReply()
                 m_channel->allDone();
                 return;
             } else {
+                // qDebug() << Q_FUNC_INFO << "WHILE BEING UNCONNECTED"; // this is one weird thing
                 m_channel->handleUnexpectedEOF();
                 return;
             }
@@ -250,7 +251,6 @@ bool QHttpProtocolHandler::sendRequest()
     if (!m_reply) {
         // heh, how should that happen!
         qWarning() << "QAbstractProtocolHandler::sendRequest() called without QHttpNetworkReply";
-        m_channel->state = QHttpNetworkConnectionChannel::IdleState;
         return false;
     }
 
diff --git a/src/network/socket/qabstractsocket.cpp b/src/network/socket/qabstractsocket.cpp
index e843597..a6949f5 100644
--- a/src/network/socket/qabstractsocket.cpp
+++ b/src/network/socket/qabstractsocket.cpp
@@ -770,7 +770,11 @@ void QAbstractSocketPrivate::canCloseNotification()
     if (isBuffered) {
         // Try to read to the buffer, if the read fail we can close the socket.
         newBytes = buffer.size();
-        if (!readFromSocket()) {
+        qint64 oldReadBufferMaxSize = readBufferMaxSize;
+        readBufferMaxSize = 0; // temporarily disable max read buffer, we want to empty the OS buffer
+        bool hadReadFromSocket = readFromSocket();
+        readBufferMaxSize = oldReadBufferMaxSize;
+        if (!hadReadFromSocket) {
             q->disconnectFromHost();
             return;
         }
diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp
index 8887f47..cdf6884 100644
--- a/src/network/ssl/qsslsocket.cpp
+++ b/src/network/ssl/qsslsocket.cpp
@@ -2300,6 +2300,14 @@ void QSslSocketPrivate::_q_errorSlot(QAbstractSocket::SocketError error)
     qDebug() << "\tstate =" << q->state();
     qDebug() << "\terrorString =" << q->errorString();
 #endif
+    // this moves encrypted bytes from plain socket into our buffer
+    if (plainSocket->bytesAvailable()) {
+        qint64 tmpReadBufferMaxSize = readBufferMaxSize;
+        readBufferMaxSize = 0; // reset temporarily so the plain sockets completely drained drained
+        transmit();
+        readBufferMaxSize = tmpReadBufferMaxSize;
+    }
+
     q->setSocketError(plainSocket->error());
     q->setErrorString(plainSocket->errorString());
     emit q->error(error);
diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp
index 13fc534..b0da336 100644
--- a/src/network/ssl/qsslsocket_openssl.cpp
+++ b/src/network/ssl/qsslsocket_openssl.cpp
@@ -1415,6 +1415,12 @@ void QSslSocketBackendPrivate::disconnected()
 {
     if (plainSocket->bytesAvailable() <= 0)
         destroySslContext();
+    else {
+        qint64 tmpReadBufferMaxSize = readBufferMaxSize;
+        readBufferMaxSize = 0; // reset temporarily so the plain socket buffer is completely drained
+        transmit();
+        readBufferMaxSize = tmpReadBufferMaxSize;
+    }
     //if there is still buffered data in the plain socket, don't destroy the ssl context yet.
     //it will be destroyed when the socket is deleted.
 }
diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
index d62e471..494a19d 100644
--- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
@@ -1049,7 +1049,7 @@ protected:
         // clean up QAbstractSocket's residue:
         while (client->bytesToWrite() > 0) {
             qDebug() << "Still having" << client->bytesToWrite() << "bytes to write, doing that now";
-            if (!client->waitForBytesWritten(2000)) {
+            if (!client->waitForBytesWritten(10000)) {
                 qDebug() << "ERROR: FastSender:" << client->error() << "cleaning up residue";
                 return;
             }
@@ -1069,7 +1069,7 @@ protected:
             measuredSentBytes += writeNextData(client, bytesToWrite);
 
             while (client->bytesToWrite() > 0) {
-                if (!client->waitForBytesWritten(2000)) {
+                if (!client->waitForBytesWritten(10000)) {
                     qDebug() << "ERROR: FastSender:" << client->error() << "during blocking write";
                     return;
                 }
@@ -7945,7 +7945,7 @@ public slots:
         m_receivedData += data;
         if (!m_parsedHeaders && m_receivedData.contains("\r\n\r\n")) {
             m_parsedHeaders = true;
-            QTimer::singleShot(qrand()%10, this, SLOT(closeDelayed())); // simulate random network latency
+            QTimer::singleShot(qrand()%60, this, SLOT(closeDelayed())); // simulate random network latency
             // This server simulates a web server connection closing, e.g. because of Apaches MaxKeepAliveRequests or KeepAliveTimeout
             // In this case QNAM needs to re-send the upload data but it had a bug which then corrupts the upload
             // This test catches that.
@@ -8051,11 +8051,12 @@ void tst_QNetworkReply::putWithServerClosingConnectionImmediately()
 
             // get the request started and the incoming socket connected
             QTestEventLoop::instance().enterLoop(10);
+            QVERIFY(!QTestEventLoop::instance().timeout());
 
             //qDebug() << "correct=" << server.m_correctUploads << "corrupt=" << server.m_corruptUploads << "expected=" <<numUploads;
 
             // Sanity check because ecause of 9c2ecf89 most replies will error out but we want to make sure at least some of them worked
-            QVERIFY(server.m_correctUploads > 5);
+            QVERIFY(server.m_correctUploads > 2);
             // Because actually important is that we don't get any corruption:
             QCOMPARE(server.m_corruptUploads, 0);

Places

File qt5-correctly-handle-connection-closed.patch of Package mingw32-libqt5-qtbase (Revision 6bdcabd9eab97cc765431140b75ef254)

Places