How to use QNetworkAccessManager without leaking memory.

JonB

@Q139
Not knowing much about electricity, your picture above is how I power my PCs. Is there anything wrong with this setup?

Q139

@JonB
For temporary solution i think its great way to power computer/s if you really needed the power and had no access to better options.

One possible bad scenario is ,if you add more PCs and increase load at some point it may act as fuse, after breaking connection short circuit and consequently breaking main fuse of the house if reasonable short circuit occurs, or just catching fire.

Maybe something corrodes over the years and starts to heat, then you will get FREE house heater out of it as well in addition to powering pc, works best on wooden house.

Connections using proper wires:

Bonnie

I replace you sleep with timer, does this change anything?

#include "form.h"
#include "ui_form.h"
#include <QtNetwork>
#include <QUrl>
#include "QProgressDialog"
#include "QMessageBox"
#include "QTextBlock"
#include "qcoreapplication.h"
Form::Form(QWidget *parent) :
    QWidget(parent),
    ui(new Ui::Form)
{
    ui->setupUi(this);

#ifndef QT_NO_SSL
    connect(&qnam, &QNetworkAccessManager::sslErrors,
            this, &Form::sslErrors);
#endif

}

Form::~Form()
{
    delete ui;
}

void Form::on_pushButton_clicked()
{
    ui->pushButton->hide();
    startRequest();
}

void Form::startRequest()
{    
    qnam.clearAccessCache();
    reply = qnam.get(QNetworkRequest(url));
    connect(reply, &QNetworkReply::finished, this, &Form::httpFinished);
}

void Form::httpFinished()
{
    reply->deleteLater();
    QTimer::singleShot(1000, this, &Form::startRequest);
}

Q139

@Bonnie Unfortunately not.

QNetworkAccessManager qnam;
QNetworkReply *reply;

void Form::startRequest()
{
    qnam.clearAccessCache();
    reply = qnam.get(QNetworkRequest(QUrl("www.microsoft.com")));
    connect(reply, &QNetworkReply::finished, this, &Form::httpFinished);
}

void Form::httpFinished()
{
     disconnect(reply, &QNetworkReply::finished, this, &Form::httpFinished);
    reply->deleteLater();
    QTimer::singleShot(1, this, &Form::startRequest);
}

This sums the core code up..
If someone plans to use it as ddos code it will probably flood your own memory up instead.

Bonnie

@Q139 Actually, I don't get a increasing memory while testing the last and the previous code of yours.
I wonder if it needs to be tested with a url that have a big size of reply data...
I'm using Qt 5.12 though...

Christian Ehrlicher

@Q139 said in How to use QNetworkAccessManager without leaking memory.:

If someone plans to use it as ddos code it will probably flood your own memory up instead.

No it won't - there is no leak in this code anymore.
The disconnect() is not needed though.

Q139

I still get leak.
alt text

Could you check if application output gives SSL error while connecting to https site?
I also had to install openSSL in addition to get it working, maybe it is related to that.
And while tested on ubuntu got no leak , but it had SSL errors in app output.
Got SSL for win7 and win10 from here, the lite edition, maybe that component causes problems.

Q139

@Bonnie Used invalid url text , like "test" also and got leak.

Q139

Install openSSL on Windows 7 or 10 and see if the app below leaks memory.

Windows ssl libraries site
direct link to download-Win64 OpenSSL v1.1.1g Light

Project file

Q139

Current solution to use QNetworkAcessManager 50 times:
I thought maybe using just once and deleting gives overhead.
This way it goes to ~100mb and back to ~20mb all the time, instead of gb+


int qnamUses=0;
QNetworkAccessManager *qnam=new QNetworkAccessManager;
QNetworkReply *reply;

void Form::startRequest()
{
    qnamUses++;
    if(qnamUses>50){
            qnam->deleteLater();

      qnam=new QNetworkAccessManager;
    }

    reply = qnam->get(QNetworkRequest(QUrl("www.microsoft.com")));
    connect(reply, &QNetworkReply::finished, this, &Form::httpFinished);
}

void Form::httpFinished()
{
  //  qnam.clearAccessCache();
    //qnam->deleteLater();
    reply->deleteLater();
    QTimer::singleShot(0, this, &Form::startRequest);
}

jsulm

@Q139 said in How to use QNetworkAccessManager without leaking memory.:

qnam=new QNetworkAccessManager;

Why do you create QNetworkAccessManager instance for each request? One can handle many requests...

Q139

@jsulm For 50 requests , then schedule for deletion. to prevent memory usage growth.
Maybe i am using something incorrectly.

SGaist

You never reset qnamUses. Therefore after 51 requests, you recreate your qnam object for every request.

Q139

Reset was fixed in test.
Is there a way to use single instance of QNetworkAccessManager for 10k+ times without drastic memory usage growth?

Christian Ehrlicher

@Q139 said in How to use QNetworkAccessManager without leaking memory.:

I still get leak.

The Task manager is no tool to measure a memory leak!

Is there a way to use single instance of QNetworkAccessManager for 10k+ times without drastic memory usage growth?

Yes, simply use it.

Q139

@Christian-Ehrlicher It ends with app filling memory and crashing if simply using.

Christian Ehrlicher

@Q139 I still don't have a valid reproducer for this behavior...

Q139

@Christian-Ehrlicher I tryed and it leaks on win 7 in vmware and win 10 native.
What version of Qt are you running? Did you also intall the openSSL?

This produces problem in my tests. Only solution i find is to delete QNetworkAccessManager after some period and recreate it.
@Q139 said in How to use QNetworkAccessManager without leaking memory.:

Install openSSL on Windows 7 or 10 and see if the app below leaks memory.

Windows ssl libraries site
direct link to download-Win64 OpenSSL v1.1.1g Light

Project file

Christian Ehrlicher

@Q139 said in How to use QNetworkAccessManager without leaking memory.:

Did you also intall the openSSL?

No need for ssl at all since your url ist http.

Christian Ehrlicher

´Please update the wait time between two requests to 100ms and see if the problem still persists. Looks like there is a race condition when the same url is queried very fast. But can't see the main reason for it currently.