QNAM: Fix CPU load for limited upload QIODevice

This fixes high CPU load for upload devices that don't generate
a constant stream of data. Their readData() function was called all the
time without returning actual data.

This was noticed when implementing an upload device that emits data in
a limited way for bandwidth limiting.

[ChangeLog][QtNetwork][QNetworkAccessManager] Fixed high CPU load when handling
POST/upload QIODevice that generates data on readyRead().

Change-Id: Iefbcb1a21d8aedef1eb11761232dd16a049018dc
Reviewed-by: Richard J. Moore <rich@kde.org>

3 files changed, 115 insertions, 0 deletions

diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp
index 56105a544b..4efb2f6a2a 100644
--- a/src/network/access/qnetworkreplyhttpimpl.cpp
+++ b/src/network/access/qnetworkreplyhttpimpl.cpp
@@ -863,6 +863,11 @@ void QNetworkReplyHttpImplPrivate::postRequest()
             forwardUploadDevice->setParent(delegate); // needed to make sure it is moved on moveToThread()
             delegate->httpRequest.setUploadByteDevice(forwardUploadDevice);
 
+            // If the device in the user thread claims it has more data, keep the flow to HTTP thread going
+            QObject::connect(uploadByteDevice.data(), SIGNAL(readyRead()),
+                             q, SLOT(uploadByteDeviceReadyReadSlot()),
+                             Qt::QueuedConnection);
+
             // From main thread to user thread:
             QObject::connect(q, SIGNAL(haveUploadData(QByteArray,bool,qint64)),
                              forwardUploadDevice, SLOT(haveDataSlot(QByteArray,bool,qint64)), Qt::QueuedConnection);
@@ -1284,6 +1289,13 @@ void QNetworkReplyHttpImplPrivate::wantUploadDataSlot(qint64 maxSize)
     // call readPointer
     qint64 currentUploadDataLength = 0;
     char *data = const_cast<char*>(uploadByteDevice->readPointer(maxSize, currentUploadDataLength));
+
+    if (currentUploadDataLength == 0) {
+        // No bytes from upload byte device. There will be bytes later, it will emit readyRead()
+        // and our uploadByteDeviceReadyReadSlot() is called.
+        return;
+    }
+
     // Let's make a copy of this data
     QByteArray dataArray(data, currentUploadDataLength);
 
@@ -1291,6 +1303,13 @@ void QNetworkReplyHttpImplPrivate::wantUploadDataSlot(qint64 maxSize)
     emit q->haveUploadData(dataArray, uploadByteDevice->atEnd(), uploadByteDevice->size());
 }
 
+void QNetworkReplyHttpImplPrivate::uploadByteDeviceReadyReadSlot()
+{
+    // Start the flow between this thread and the HTTP thread again by triggering a upload.
+    wantUploadDataSlot(1024);
+}
+
+
 /*
     A simple web page that can be used to test us: http://www.procata.com/cachetest/
  */
diff --git a/src/network/access/qnetworkreplyhttpimpl_p.h b/src/network/access/qnetworkreplyhttpimpl_p.h
index d21659ce82..06a5383ae4 100644
--- a/src/network/access/qnetworkreplyhttpimpl_p.h
+++ b/src/network/access/qnetworkreplyhttpimpl_p.h
@@ -129,6 +129,7 @@ public:
     Q_PRIVATE_SLOT(d_func(), void resetUploadDataSlot(bool *r))
     Q_PRIVATE_SLOT(d_func(), void wantUploadDataSlot(qint64))
     Q_PRIVATE_SLOT(d_func(), void sentUploadDataSlot(qint64))
+    Q_PRIVATE_SLOT(d_func(), void uploadByteDeviceReadyReadSlot())
     Q_PRIVATE_SLOT(d_func(), void emitReplyUploadProgress(qint64, qint64))
     Q_PRIVATE_SLOT(d_func(), void _q_cacheSaveDeviceAboutToClose())
 
@@ -300,6 +301,9 @@ public:
     void wantUploadDataSlot(qint64);
     void sentUploadDataSlot(qint64);
 
+    // From user's QNonContiguousByteDevice
+    void uploadByteDeviceReadyReadSlot();
+
     Q_DECLARE_PUBLIC(QNetworkReplyHttpImpl)
 };
 
diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
index 0509eb9a77..480eeecb63 100644
--- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
@@ -461,6 +461,8 @@ private Q_SLOTS:
     void backgroundRequestConnectInBackground();
 #endif
 
+    void putWithRateLimiting();
+
     // NOTE: This test must be last!
     void parentingRepliesToTheApp();
 private:
@@ -7776,6 +7778,96 @@ void tst_QNetworkReply::backgroundRequestConnectInBackground()
 }
 #endif
 
+class RateLimitedUploadDevice : public QIODevice
+{
+    Q_OBJECT
+public:
+    QByteArray data;
+    QBuffer buffer;
+    qint64 read;
+    qint64 bandwidthQuota;
+    QTimer timer;
+
+    RateLimitedUploadDevice(QByteArray d) : QIODevice(),data(d),read(0),bandwidthQuota(0) {
+        buffer.setData(data);
+        buffer.open(QIODevice::ReadOnly);
+        timer.setInterval(200);
+        QObject::connect(&timer, SIGNAL(timeout()), this, SLOT(timeoutSlot()));
+        timer.start();
+    }
+
+    virtual qint64 writeData(const char* , qint64 ) {
+        Q_ASSERT(false);
+        return 0;
+    }
+
+    virtual qint64 readData(char* data, qint64 maxlen) {
+        //qDebug() << Q_FUNC_INFO << maxlen << bandwidthQuota;
+        maxlen = qMin(maxlen, buffer.bytesAvailable());
+        maxlen = qMin(maxlen, bandwidthQuota);
+        if (maxlen <= 0) {  // no quota or at end
+            return 0;
+        }
+        bandwidthQuota -= maxlen; // reduce quota
+
+        qint64 ret = buffer.read(data, maxlen);
+        if (ret == -1) {
+            return -1;
+        }
+        read += ret;
+        //qDebug() << Q_FUNC_INFO << maxlen << bandwidthQuota << read << ret << buffer.bytesAvailable();
+        return ret;
+    }
+    virtual bool atEnd() const {
+        return buffer.atEnd();
+    }
+    virtual qint64 size() const{
+        return data.length();
+    }
+    qint64 bytesAvailable() const
+    {
+        return buffer.bytesAvailable() + QIODevice::bytesAvailable();
+    }
+    virtual bool isSequential() const{ // random access, we can seek
+        return false;
+    }
+    virtual bool seek ( qint64 pos ) {
+        return buffer.seek(pos);
+    }
+protected slots:
+    void timeoutSlot() {
+        //qDebug() << Q_FUNC_INFO;
+        bandwidthQuota = 8*1024; // fill quota
+        emit readyRead();
+    }
+};
+
+void tst_QNetworkReply::putWithRateLimiting()
+{
+    QFile reference(testDataDir + "/rfc3252.txt");
+    reference.open(QIODevice::ReadOnly);
+    QByteArray data = reference.readAll();
+    QVERIFY(data.length() > 0);
+
+    QUrl url = QUrl::fromUserInput("http://" + QtNetworkSettings::serverName()+ "/qtest/cgi-bin/echo.cgi?");
+
+    QNetworkRequest request(url);
+    QNetworkReplyPtr reply;
+
+    RateLimitedUploadDevice rateLimitedUploadDevice(data);
+    rateLimitedUploadDevice.open(QIODevice::ReadOnly);
+
+    RUN_REQUEST(runCustomRequest(request, reply,QByteArray("POST"), &rateLimitedUploadDevice));
+    QCOMPARE(reply->error(), QNetworkReply::NoError);
+    QCOMPARE(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200);
+
+    QByteArray uploadedData = reply->readAll();
+    QCOMPARE(uploadedData.length(), data.length());
+    QCOMPARE(uploadedData, data);
+}
+
+
+
 // NOTE: This test must be last testcase in tst_qnetworkreply!
 void tst_QNetworkReply::parentingRepliesToTheApp()
 {