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

robsparrow

I have huge file to download. I launch code below with GUI button click. "readyRead" signal is called sometimes once and thats all. it keeps downloading whole file in memory until app crashes.
I guess its related to UI thread. I tried move this code to class extending QThread but same issue - "readyRead" is not called there also. How this code should be extended to be able get every signal for "readyRead" and execute "reply->readAll()" to release data from memory?

QString url = "http://localhost/backup-2022.tar.gz"; // 48 GB file

    QNetworkAccessManager qnam;
    QNetworkRequest request = QNetworkRequest(url);
    QNetworkReply *reply = qnam.get(request);
    
    QEventLoop loop;
    connect(reply, &QNetworkReply::readyRead, [&]{
        // this is not called, memory is filled, cant release received data
        reply->readAll();
    });
    loop.exec();

Thank You guys, hopefully someone can help me!

Christian Ehrlicher

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

Thank You guys, hopefully someone can help me!

As always - use as intended with signals/slots and without a local eventloop.

robsparrow

@Christian-Ehrlicher I tried move it to its own class, with signal/slot defined, but its again not working without QEventLoop, and with it its again filling memory until crash, without readyRead slot execution

fileloader.h

// fileloader.h
#include <QDebug>
#include <QObject>
#include <QNetworkAccessManager>
#include <QNetworkRequest>
#include <QNetworkReply>
#include <QEventLoop>

class fileloader : public QObject
{
    Q_OBJECT
public:
    fileloader();
    void loadcontent();
    QNetworkReply *reply;

private slots:
    void readyRead();
};

fileloader.cpp

// fileloader.cpp
#include "fileloader.h"

fileloader::fileloader()
{

}

void fileloader::loadcontent() {

    QString url = "http://localhost/backup-2022.tar.gz";
    
    QNetworkAccessManager qnam;
    QNetworkRequest request = QNetworkRequest(url);
    reply = qnam.get(request);

    // if I remove this, its not even starting request to server
    // with event loop request is started but its again filling memory until crash
    QEventLoop loop;
    connect(reply, SIGNAL(readyRead()), this, SLOT(readyRead()));
    loop.exec();
}

void fileloader::readyRead()
{
    reply->readAll();
}

mainwindow.cpp

// in mainwindow.cpp
void MainWindow::on_fileload_clicked()
{
    fileloader fl;
    fl.loadcontent();
}

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

Thank You!

mpergand

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

QNetworkAccessManager qnam;

QNetworkAccessManager* qnam=new QNetworkAccessManager(this);

No need of any runloop.

? Offline

fileloader.h

// fileloader.h
class fileloader : public QObject
{
    Q_OBJECT
public:
    fileloader();
    void loadcontent();
    QNetworkReply *reply;

private slots:
    void readyRead();

private:
   QNetworkAccessManager m_qnam ;
};

fileloader.cpp

// fileloader.cpp

void fileloader::loadcontent() {

QString url = "http://localhost/backup-2022.tar.gz";

// QNetworkAccessManager has nothing to do here !!!
QNetworkRequest request = QNetworkRequest(url);
reply = m_qnam.get(request);
connect (reply, &QNetworkReply::readyRead, this, &fileloader::readyRead) ;

}

void fileloader::readyRead()
{
reply->readAll();
}

Your QNetworkAccessManager should be private member of the fileloader class and should live as long as this class lives.

So either you declare it as a pointer and you instanciate it in the constructor, or you just declare it as a simple member and that's good.

I don't understand how it can overflow your memory if the network manager is destroyed after barely starting the download.

robsparrow

@ankou29666 , @mpergand, @Christian-Ehrlicher I am still strugling to solve this issue. Moved it to private member. As you can see in screenshot as soon as it starts download file, memory starts increasing until I terminated exection of app.

Here is full project in .zip with files only 10 KB (just UI and fileloader .h, .cpp files) - https://we.tl/t-aRSHEv8ETa

memory fill.PNG

Thank You for helping guys!

ChrisW67

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

I have huge file to download.

Just how big is huge?

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

? Offline

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:

calling "currentDownloads[0]->readAll()" just to flush data - does not releases memory
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(); ?

? Offline

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

? Offline

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