HTTP internals: do not open too many sockets when preconnecting

Each pair of (normal request, preconnect request) requires only one
socket. E.g. if there is 1 preconnect request in-flight and 2 normal
requests, we need only 2 sockets in total, and not 3.

Therefore, we need to keep track of whether a request is
preconnecting or a normal one.

Task-number: QTBUG-31594
Change-Id: If92ccc35abadfa6090d64ee92bd466615909c94c
Reviewed-by: Richard J. Moore <rich@kde.org>

src/network/access/qhttpnetworkconnection.cpp

@@ -85,6 +85,7 @@ QHttpNetworkConnectionPrivate::QHttpNetworkConnectionPrivate(const QString &host
 #ifndef QT_NO_NETWORKPROXY
   , networkProxy(QNetworkProxy::NoProxy)
 #endif
+  , preConnectRequests(0)
 {
     channels = new QHttpNetworkConnectionChannel[channelCount];
 }
@@ -96,6 +97,7 @@ QHttpNetworkConnectionPrivate::QHttpNetworkConnectionPrivate(quint16 channelCoun
 #ifndef QT_NO_NETWORKPROXY
   , networkProxy(QNetworkProxy::NoProxy)
 #endif
+  , preConnectRequests(0)
 {
     channels = new QHttpNetworkConnectionChannel[channelCount];
 }
@@ -541,6 +543,9 @@ QHttpNetworkReply* QHttpNetworkConnectionPrivate::queueRequest(const QHttpNetwor
     reply->d_func()->connectionChannel = &channels[0]; // will have the correct one set later
     HttpMessagePair pair = qMakePair(request, reply);
 
+    if (request.isPreConnect())
+        preConnectRequests++;
+
     switch (request.priority()) {
     case QHttpNetworkRequest::HighPriority:
         highPriorityQueue.prepend(pair);
@@ -925,15 +930,24 @@ void QHttpNetworkConnectionPrivate::_q_startNextRequest()
     // If there is not already any connected channels we need to connect a new one.
     // We do not pair the channel with the request until we know if it is
     // connected or not. This is to reuse connected channels before we connect new once.
-    int queuedRequest = highPriorityQueue.count() + lowPriorityQueue.count();
+    int queuedRequests = highPriorityQueue.count() + lowPriorityQueue.count();
-    for (int i = 0; i < channelCount; ++i) {
+
+    // in case we have in-flight preconnect requests and normal requests,
+    // we only need one socket for each (preconnect, normal request) pair
+    int neededOpenChannels = queuedRequests;
+    if (preConnectRequests > 0) {
+        int normalRequests = queuedRequests - preConnectRequests;
+        neededOpenChannels = qMax(normalRequests, preConnectRequests);
+    }
+    for (int i = 0; i < channelCount && neededOpenChannels > 0; ++i) {
         bool connectChannel = false;
         if (channels[i].socket) {
             if ((channels[i].socket->state() == QAbstractSocket::ConnectingState)
                     || (channels[i].socket->state() == QAbstractSocket::HostLookupState)
                     || channels[i].pendingEncrypt) // pendingEncrypt == "EncryptingState"
-                queuedRequest--;
+                neededOpenChannels--;
-            if ( queuedRequest <=0 )
+
+            if (neededOpenChannels <= 0)
                 break;
             if (!channels[i].reply && !channels[i].isSocketBusy() && (channels[i].socket->state() == QAbstractSocket::UnconnectedState))
                 connectChannel = true;
@@ -947,11 +961,8 @@ void QHttpNetworkConnectionPrivate::_q_startNextRequest()
             else if (networkLayerState == IPv6)
                 channels[i].networkLayerPreference = QAbstractSocket::IPv6Protocol;
             channels[i].ensureConnection();
-            queuedRequest--;
+            neededOpenChannels--;
         }
-
-        if ( queuedRequest <=0 )
-            break;
     }
 }
 
@@ -1272,6 +1283,11 @@ void QHttpNetworkConnection::ignoreSslErrors(const QList<QSslError> &errors, int
 
 #endif //QT_NO_SSL
 
+void QHttpNetworkConnection::preConnectFinished()
+{
+    d_func()->preConnectRequests--;
+}
+
 #ifndef QT_NO_NETWORKPROXY
 // only called from QHttpNetworkConnectionChannel::_q_proxyAuthenticationRequired, not
 // from QHttpNetworkConnectionChannel::handleAuthenticationChallenge

src/network/access/qhttpnetworkconnection_p.h

@@ -131,6 +131,8 @@ public:
     void setSslContext(QSharedPointer<QSslContext> context);
 #endif
 
+    void preConnectFinished();
+
 private:
     Q_DECLARE_PRIVATE(QHttpNetworkConnection)
     Q_DISABLE_COPY(QHttpNetworkConnection)
@@ -239,6 +241,8 @@ public:
     QList<HttpMessagePair> highPriorityQueue;
     QList<HttpMessagePair> lowPriorityQueue;
 
+    int preConnectRequests;
+
 #ifndef QT_NO_SSL
     QSharedPointer<QSslContext> sslContext;
 #endif

src/network/access/qhttpnetworkconnectionchannel.cpp

@@ -214,6 +214,7 @@ bool QHttpNetworkConnectionChannel::sendRequest()
             state = QHttpNetworkConnectionChannel::IdleState;
             reply->d_func()->state = QHttpNetworkReplyPrivate::AllDoneState;
             allDone();
+            connection->preConnectFinished(); // will only decrease the counter
             reply = 0; // so we can reuse this channel
             return true; // we have a working connection and are done
         }

src/network/access/qhttpnetworkrequest.cpp

@@ -49,7 +49,8 @@ QT_BEGIN_NAMESPACE
 QHttpNetworkRequestPrivate::QHttpNetworkRequestPrivate(QHttpNetworkRequest::Operation op,
         QHttpNetworkRequest::Priority pri, const QUrl &newUrl)
     : QHttpNetworkHeaderPrivate(newUrl), operation(op), priority(pri), uploadByteDevice(0),
-      autoDecompress(false), pipeliningAllowed(false), withCredentials(true)
+      autoDecompress(false), pipeliningAllowed(false), withCredentials(true),
+      preConnect(false)
 {
 }
 
@@ -64,6 +65,7 @@ QHttpNetworkRequestPrivate::QHttpNetworkRequestPrivate(const QHttpNetworkRequest
     customVerb = other.customVerb;
     withCredentials = other.withCredentials;
     ssl = other.ssl;
+    preConnect = other.preConnect;
 }
 
 QHttpNetworkRequestPrivate::~QHttpNetworkRequestPrivate()
@@ -75,7 +77,8 @@ bool QHttpNetworkRequestPrivate::operator==(const QHttpNetworkRequestPrivate &ot
     return QHttpNetworkHeaderPrivate::operator==(other)
         && (operation == other.operation)
         && (ssl == other.ssl)
-        && (uploadByteDevice == other.uploadByteDevice);
+        && (uploadByteDevice == other.uploadByteDevice)
+        && (preConnect == other.preConnect);
 }
 
 QByteArray QHttpNetworkRequestPrivate::methodName() const
@@ -205,6 +208,15 @@ void QHttpNetworkRequest::setSsl(bool s)
     d->ssl = s;
 }
 
+bool QHttpNetworkRequest::isPreConnect() const
+{
+    return d->preConnect;
+}
+void QHttpNetworkRequest::setPreConnect(bool preConnect)
+{
+    d->preConnect = preConnect;
+}
+
 qint64 QHttpNetworkRequest::contentLength() const
 {
     return d->contentLength();

src/network/access/qhttpnetworkrequest_p.h

@@ -120,6 +120,9 @@ public:
     bool isSsl() const;
     void setSsl(bool);
 
+    bool isPreConnect() const;
+    void setPreConnect(bool preConnect);
+
     void setUploadByteDevice(QNonContiguousByteDevice *bd);
     QNonContiguousByteDevice* uploadByteDevice() const;
 
@@ -151,6 +154,7 @@ public:
     bool pipeliningAllowed;
     bool withCredentials;
     bool ssl;
+    bool preConnect;
 };
 
 

src/network/access/qnetworkreplyhttpimpl.cpp

@@ -635,6 +635,9 @@ void QNetworkReplyHttpImplPrivate::postRequest()
     q->setAttribute(QNetworkRequest::ConnectionEncryptedAttribute, ssl);
     httpRequest.setSsl(ssl);
 
+    bool preConnect = (scheme == QLatin1String("preconnect-http")
+                       || scheme == QLatin1String("preconnect-https"));
+    httpRequest.setPreConnect(preConnect);
 
 #ifndef QT_NO_NETWORKPROXY
     QNetworkProxy transparentProxy, cacheProxy;

tests/benchmarks/network/access/qnetworkreply/tst_qnetworkreply.cpp

@@ -463,6 +463,7 @@ private slots:
 #ifndef QT_NO_SSL
     void echoPerformance_data();
     void echoPerformance();
+    void preConnectEncrypted_data();
     void preConnectEncrypted();
 #endif
 
@@ -476,6 +477,7 @@ private slots:
     void httpDownloadPerformanceDownloadBuffer();
     void httpsRequestChain();
     void httpsUpload();
+    void preConnect_data();
     void preConnect();
 
 private:
@@ -548,6 +550,13 @@ void tst_qnetworkreply::echoPerformance()
     }
 }
 
+void tst_qnetworkreply::preConnectEncrypted_data()
+{
+    QTest::addColumn<int>("sleepTime");
+    QTest::newRow("2secs") << 2000; // to start a new request after preconnecting is done
+    QTest::newRow("100ms") << 100; // to start a new request while preconnecting is in-flight
+}
+
 void tst_qnetworkreply::preConnectEncrypted()
 {
     QString hostName = QLatin1String("www.google.com");
@@ -579,8 +588,9 @@ void tst_qnetworkreply::preConnectEncrypted()
     manager.clearAccessCache();
 
     // now try to make the connection beforehand
+    QFETCH(int, sleepTime);
     manager.connectToHostEncrypted(hostName);
-    QTestEventLoop::instance().enterLoop(2);
+    QTestEventLoop::instance().enterLoopMSecs(sleepTime);
 
     // now make another request and hopefully use the existing connection
     QPair<QNetworkReply *, qint64> preConnectResult = runGetRequest(&manager, request);
@@ -917,6 +927,11 @@ void tst_qnetworkreply::httpsUpload()
     }
 }
 
+void tst_qnetworkreply::preConnect_data()
+{
+    preConnectEncrypted_data();
+}
+
 void tst_qnetworkreply::preConnect()
 {
     QString hostName = QLatin1String("www.google.com");
@@ -948,8 +963,9 @@ void tst_qnetworkreply::preConnect()
     manager.clearAccessCache();
 
     // now try to make the connection beforehand
+    QFETCH(int, sleepTime);
     manager.connectToHost(hostName);
-    QTestEventLoop::instance().enterLoop(2);
+    QTestEventLoop::instance().enterLoopMSecs(sleepTime);
 
     // now make another request and hopefully use the existing connection
     QPair<QNetworkReply *, qint64> preConnectResult = runGetRequest(&manager, request);