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

[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.

[JonB]

@robsparrow
The results shown by @ChrisW67 under Linux are indeed what would be expected.

One thing to check: your earlier Memory usage screenshot from Task Manager > Performance looks like it's showing for whole machine? Can you verify whether that memory is being used by your Qt process versus whether it is attributed to something else, like the OS itself?

[A Former User]

@JonB that wasn't my graphs !!! All I said is I was wondering what the behavior is for big files. Personally I'm not concerned by such file sizes, and haven't yet reached the point where I will have to deal with bigger file sizes (and I'll still be well below RAM quantity anyways).

I have already seen inaccurate documentations ;) so sometimes I take them a little cautiously.

[JonB]

@ankou29666 My fault, I meant to address that to the OP @robsparrow, corrected.

[J.Hilk]

@JonB its the new forum style at fault here,

@ankou29666 and @robsparrow have the same color back ground image.

It's also the only thing I tend to look at, at first glance at least

[JonB]

@J-Hilk
I also find the new behaviour here when typing @... since the forum update confusing.... Though it's probably not to blame for my fault above :)

[J.Hilk]

@JonB In think that got reverted!