QNetworkReply readyRead is not called, memory filling to max until crash

[jsulm]

@robsparrow said in QNetworkReply readyRead is not called, memory filling to max until crash:

Can You or someone else point me forward - whats wrong now and what should be changed in this structure?

As already pointed out several times: do not use any local event loops! Use signal/slots. Qt is an asynchronous framework and should use it as such. There are also examples for Qt networking available.
If you have to download a huge file then do not hold whole downloaded data in memory, instead write directly to file every time you get new package.

[robsparrow]

@ChrisW67 - 48 GB file
@jsulm - that is what I am trying to achieve, write/handle each package of data with readyRead signal.

I tried also this example - https://code.qt.io/cgit/qt/qtbase.git/tree/examples/network/download?h=6.4

and added just this:

connect(reply, &QNetworkReply::readyRead,
            this, &DownloadManager::readyRead);

and

void DownloadManager::readyRead() {
    currentDownloads[0]->readAll(); // const 0
}

And happens exactly same issue what previously - readyRead is called few times (less than 5) and then memory starts filling.

At this point I am thinking that there might be issue with Qt, because nothing is really working, all solutions is filling memory, even their example. Maybe this test case should be reported. Currently I am using Qt 6.4.2

[jsulm]

@robsparrow I suggested that you write incoming data directly into a file instead of buffering it in RAM - is this what you are doing now or not?

[J.Hilk]

@robsparrow said in QNetworkReply readyRead is not called, memory filling to max until crash:

At this point I am thinking that there might be issue with Qt

No, most certainly not.

first of all, either delete your QNetworkReply after use, or set it to be auto deleted.
https://doc.qt.io/qt-6/qnetworkaccessmanager.html#setAutoDeleteReplies

than, as said previously, write your file to disk, from what you have shown so far, you're still not doing that, but rather keep all data in memory

[A Former User]

according to qnetworkreply documentation

void QNetworkReply::setReadBufferSize(qint64 size)
Sets the size of the read buffer to be size bytes. The read buffer is the buffer that holds data that is being downloaded off the network, before it is read with QIODevice::read(). Setting the buffer size to 0 will make the buffer unlimited in size.

QNetworkReply will try to stop reading from the network once this buffer is full (i.e., bytesAvailable() returns size or more), thus causing the download to throttle down as well. If the buffer is not limited in size, QNetworkReply will try to download as fast as possible from the network.

Unlike QAbstractSocket::setReadBufferSize(), QNetworkReply cannot guarantee precision in the read buffer size. That is, bytesAvailable() can return more than size.

The problem is that documentation doesn't tell the default value of buffer size.
I think it might be usefull (at least to try) to set explicitly a size for the buffer.

I have no idea how this behaves by default with huge files.

[jsulm]

@ankou29666 Can you please tell us exactly (show the code) what you are doing with incoming data? You still refuse to tell us whether you're writing data directly to a file or accumulating it in RAM. If you accumulate all incoming data in RAM, then of course you will get out of memory if you're sending huge amount of data and this has nothing to do with Qt...
And the documentation you posted has nothing to do with what YOU are doing with incoming data.

[JonB]

@J-Hilk said in QNetworkReply readyRead is not called, memory filling to max until crash:

than, as said previously, write your file to disk, from what you have shown so far, you're still not doing that, but rather keep all data in memory

In what way? The OP has

void DownloadManager::readyRead() {
    currentDownloads[0]->readAll(); // const 0
}

Every time this is called it should read and discard whatever has arrived so far. Then one would hope that Qt code is not buffering the whole file into memory but rather ca re-use memory allocated. Btw, @ankou29666 have you checked this is getting called regularly?

Maybe you are right and writing to a file will make a difference, but I don't see why.

[robsparrow]

@J-Hilk , @jsulm if I am reading documentation ( https://doc.qt.io/qt-6/qnetworkreply.html#details ) correctly then executing "reply->readAll()" should be returning data and flushing it from memory. its not even about writing it to file anymore if its just failing to release data from memory.

So far:

1. calling "currentDownloads[0]->readAll()" just to flush data - does not releases memory
2. writing each chunk of data to file "dfile->write(currentDownloads[0]->readAll());" - also does not release memory

---

Here I am trying to download it in Qt example code- "dfile->write(currentDownloads[0]->readAll());"

Added also - "manager.setAutoDeleteReplies(true);" @J-Hilk

in readyRead slot I added n++ with qdebug, few times I just see it called 2 times, some times 4..

Here is full code based on Qt download example without UI:

#include <QtCore>
#include <QtNetwork>

#include <cstdio>

QT_BEGIN_NAMESPACE
class QSslError;
QT_END_NAMESPACE

using namespace std;

class DownloadManager: public QObject
{
    Q_OBJECT
    QNetworkAccessManager manager;
    QList<QNetworkReply *> currentDownloads;

public:
    DownloadManager();
    void doDownload(const QUrl &url);
    static QString saveFileName(const QUrl &url);
    bool saveToDisk(const QString &filename, QIODevice *data);
    static bool isHttpRedirect(QNetworkReply *reply);

public slots:
    void execute();
    void downloadFinished(QNetworkReply *reply);
    void readyRead();
    void sslErrors(const QList<QSslError> &errors);
private:
    int n = 0;
    QFile *dfile;
};

DownloadManager::DownloadManager()
{
    manager.setAutoDeleteReplies(true);
    connect(&manager, &QNetworkAccessManager::finished,
            this, &DownloadManager::downloadFinished);

    dfile = new QFile("failtodownload.tar.gz");
    dfile->open(QIODevice::WriteOnly);
}

void DownloadManager::doDownload(const QUrl &url)
{
    QNetworkRequest request(url);
    QNetworkReply *reply = manager.get(request);

#if QT_CONFIG(ssl)
    connect(reply, &QNetworkReply::sslErrors,
            this, &DownloadManager::sslErrors);
#endif

    connect(reply, &QNetworkReply::readyRead,
            this, &DownloadManager::readyRead);

    currentDownloads.append(reply);
}

QString DownloadManager::saveFileName(const QUrl &url)
{
    QString path = url.path();
    QString basename = QFileInfo(path).fileName();

    if (basename.isEmpty())
        basename = "download";

    if (QFile::exists(basename)) {
        // already exists, don't overwrite
        int i = 0;
        basename += '.';
        while (QFile::exists(basename + QString::number(i)))
            ++i;

        basename += QString::number(i);
    }

    return basename;
}

bool DownloadManager::saveToDisk(const QString &filename, QIODevice *data)
{
    QFile file(filename);
    if (!file.open(QIODevice::WriteOnly)) {
        fprintf(stderr, "Could not open %s for writing: %s\n",
                qPrintable(filename),
                qPrintable(file.errorString()));
        return false;
    }

    file.write(data->readAll());
    file.close();

    return true;
}

bool DownloadManager::isHttpRedirect(QNetworkReply *reply)
{
    int statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
    return statusCode == 301 || statusCode == 302 || statusCode == 303
           || statusCode == 305 || statusCode == 307 || statusCode == 308;
}

void DownloadManager::execute()
{
    // hardcoded url
    QUrl url = QUrl::fromEncoded("http://localhost/backup-2022.tar.gz");
    doDownload(url);
}

void DownloadManager::sslErrors(const QList<QSslError> &sslErrors)
{
#if QT_CONFIG(ssl)
    for (const QSslError &error : sslErrors)
        fprintf(stderr, "SSL error: %s\n", qPrintable(error.errorString()));
#else
    Q_UNUSED(sslErrors);
#endif
}

void DownloadManager::downloadFinished(QNetworkReply *reply)
{
    QUrl url = reply->url();
    if (reply->error()) {
        fprintf(stderr, "Download of %s failed: %s\n",
                url.toEncoded().constData(),
                qPrintable(reply->errorString()));
    } else {
        if (isHttpRedirect(reply)) {
            fputs("Request was redirected.\n", stderr);
        } else {
            QString filename = saveFileName(url);
            if (saveToDisk(filename, reply)) {
                printf("Download of %s succeeded (saved to %s)\n",
                       url.toEncoded().constData(), qPrintable(filename));
            }
        }
    }

    currentDownloads.removeAll(reply);
    reply->deleteLater();

    if (currentDownloads.isEmpty()) {
        // all downloads finished
        QCoreApplication::instance()->quit();
    }

    dfile->close();
}

void DownloadManager::readyRead() {
    n++;
    qDebug() << "readyRead " << n;

    // hardcode to first url
    // currentDownloads[0]->readAll() // just calling this should release data from memory.
    dfile->write(currentDownloads[0]->readAll());
}

int main(int argc, char **argv)
{
    QCoreApplication app(argc, argv);

    DownloadManager manager;
    QTimer::singleShot(0, &manager, SLOT(execute()));

    app.exec();
}

[JonB]

@robsparrow said in QNetworkReply readyRead is not called, memory filling to max until crash:

then executing "reply->readAll()" should be returning data and flushing it from memory. its not even about writing it to file anymore

Yes as I wrote above I see it this way too. FWIW you might try setReadBufferSize(4096) just to see whether that throttles how much memory is used?

[jsulm]

@robsparrow OK, now I understand.
Is there a difference if you comment out currentDownloads[0]->readAll(); ?

[A Former User]

@jsulm said in QNetworkReply readyRead is not called, memory filling to max until crash:

@ankou29666 Can you please tell us exactly (show the code) what you are doing with incoming data? You still refuse to tell us whether you're writing data directly to a file or accumulating it in RAM. If you accumulate all incoming data in RAM, then of course you will get out of memory if you're sending huge amount of data and this has nothing to do with Qt...
And the documentation you posted has nothing to do with what YOU are doing with incoming data.

I AM NOT THE ONE WITH THE PROBLEM
please be a little more careful about it.
just trying to help the way I can. I don't download gigabyte files (1kB file is big file in my case so far). All I'm saying is that I wonder what is the default buffer size, because when explicitly set to zero, the buffer size is limited only by the device's memory.

[jsulm]

@ankou29666 Sorry, I replied to you by mistake.

[A Former User]

@jsulm I had guessed. No worry this can happen to anyone ;)

[robsparrow]

Solved!
reply->setReadBufferSize(4096) did help, now readyRead signal is called and I can read data and release it from memory.

If here is anyone with power to do something with documentation then probably here https://doc.qt.io/qt-6/qnetworkreply.html#details should be added that readyRead event is directly controlled by setReadBufferSize(), because now documentation just says "Whenever more data is received from the network and processed, the readyRead() signal is emitted."

Also readyRead is not working as expected if we follow to this part of documentation - https://doc.qt.io/qt-6/qnetworkreply.html#setReadBufferSize
"Setting the buffer size to 0 will make the buffer unlimited in size." as default value is 0. I would not expect even 1 signal for readyRead, but I was getting from 1-7 signals even in first seconds while file downloading until memory was filled or app was not responsive. but file was not fully downloaded while those signals was sent. So why there is such random behaviour on this signal..

[Christian Ehrlicher]

@robsparrow said in QNetworkReply readyRead is not called, memory filling to max until crash:

So why there is such random behaviour on this signal..

There is no random behavior on this signal:

"This signal is emitted once every time new data is available for reading from the device's current read channel. It will only be emitted again once new data is available, such as when a new payload of network data has arrived on your network socket, or when a new block of data has been appended to your device."

Nothing about buffer sizes here

[A Former User]

@Christian-Ehrlicher said in QNetworkReply readyRead is not called, memory filling to max until crash:

Nothing about buffer sizes here

I think it might. When downloading short files, the signal will only be sent once the request is totally complete. I don't know how it works in internal but I wouldn't be surprised that readyRead signal would be fired only once the bytecount in the buffer matches the bytecount declared in the reply's http header.

I don't know if I'm right or wrong, but in that case it would explain why he never received the readyRead signal if he gets a memory overflow before the file is completely downloaded.

[Christian Ehrlicher]

@ankou29666 said in QNetworkReply readyRead is not called, memory filling to max until crash:

I think it might. When downloading short files, the signal will only be sent once the request is totally complete. I don't know how it works in internal but I wouldn't be surprised that readyRead signal would be fired only once the bytecount in the buffer matches the bytecount declared in the reply's http header.

No - it's clearly written: "This signal is emitted once every time new data is available for reading from the device's read channel"

[A Former User]

@Christian-Ehrlicher this statement is obviously wrong.

by the way what do they actually mean by "new data" ? and same about "availability".

[Christian Ehrlicher]

@ankou29666 said in QNetworkReply readyRead is not called, memory filling to max until crash:

this statement is obviously wrong.

Ok, no need to discuss further here...

Qt waits for an event from the OS and emits a queued signal readyRead() so it depends on the current OS + CPU usage...

[ChrisW67]

The OP's unmodified code, on my machine with just 16GB of RAM: retrieving a local file:

$ curl -I http://localhost/backup-2022.tar.gz
HTTP/1.1 200 OK
Server: nginx/1.18.0 (Ubuntu)
Date: Tue, 28 Feb 2023 07:53:57 GMT
Content-Type: application/octet-stream
Content-Length: 15072231424
Last-Modified: Tue, 28 Feb 2023 07:42:03 GMT
Connection: keep-alive
ETag: "63fdb04b-382600000"
Accept-Ranges: bytes

• Emits readyRead thousands of times, reads, and discards the data as expected and documented.
• Finishes cleanly
• Barely cracks 1% of RAM used
• Linux Qt 6.4.2 and 5.15.2

Exhausting RAM without completing is environmental, a Windows platform issue, or web server oddity (is it CPU bound for example).

The only time I had a lone readyRead() call was the web server returning an HTTP 403 (PEBKAC).

Initially, downloading multiple times in the same run consumed an extra memory chunk each time because the QNetworkReply is not deleted. Fixing that fixes this growth.