Memory leak then using QNetworkProxy

[denys]

3 years ago was similar question https://forum.qt.io/topic/69102/memory-leak-in-qnetworkproxy-should-i-post-on-bugtracker, but it is discontinued, so i start personal thread
my app download small files many time. If it's don't use proxy - memory usage is stable, but if I call setProxy with proxy type QNetworkProxy::HttpProxy - i have stable increasing of memory usage. It's reproduced on Windows 10, and on ubuntu linux.
There is my test source code.

void MainWindow::httpTestFinished()
{
    httpTestRequestSuccess = true;
    if(pHttpTestEventLoop)
        pHttpTestEventLoop->quit();
    replyTest->deleteLater();
    replyTest = nullptr;
}

void MainWindow::httpTestReadyRead()
{
    httpTestAnswer += replyTest->readAll();
}

void MainWindow::on_pushButton_clicked()
{
    pHttpTestEventLoop = new QEventLoop;
    for(int i=0;i<500;++i)
    {
        httpTestRequestSuccess = false;
        httpTestAnswer = QByteArray();
        const QUrl url = QUrl::fromUserInput( "https://small page to test" );
        QNetworkAccessManager qNAM;
        /*qNAM.setProxy(
                    QNetworkProxy(
                        QNetworkProxy::NoProxy
                        ));*/
        qNAM.setProxy(
                    QNetworkProxy(
                        QNetworkProxy::HttpProxy,
                        "proxy-host",
                        8000,
                        "uname",
                        "password"
                        ));

        QNetworkRequest request(url);
        //request.setRawHeader("User-Agent", httpConnectionInfo.agent.toLatin1());
        replyTest = qNAM.get(request) ;
        connect(replyTest, &QNetworkReply::finished, this, &MainWindow::httpTestFinished);
        connect(replyTest, &QIODevice::readyRead, this, &MainWindow::httpTestReadyRead);
        connect(replyTest, &QNetworkReply::finished, pHttpTestEventLoop, &QEventLoop::quit);
        QTimer timer(this);
        timer.setSingleShot(true);
        timer.setInterval(1000);
        timer.start();
        connect(&timer, &QTimer::timeout, pHttpTestEventLoop, &QEventLoop::quit);
        pHttpTestEventLoop->exec();
        timer.stop();
        if(replyTest)
        {
            replyTest->deleteLater();
            replyTest = nullptr;
        }
        ui->label->setText( QString::number(i));
    }
    pHttpTestEventLoop->deleteLater();
    QMessageBox::information(0, "Done", "Done");
}

On every click button - Memory usage increased about 1MB
Important conditions - https and proxy
Qt version 5.13.0
Post on bugtracker. https://bugreports.qt.io/browse/QTBUG-77623
There is project ready to test )

[mrjj]

Hi
If you are fairly sure it seems like a bug then please do.
and please post the link here
Small note.
A fully runnable example is better so the maintainer does not have to
create default project and copy your functions to it.
So when you add the zip file to the bug report , It would be great
if it included the pro file, main.cpp and mainwindow.x
so its just to open project and run to see.

[Pablo J. Rogina]

@denys said in Memory leak then using QNetworkProxy:

void MainWindow::httpTestFinished()
{
httpTestRequestSuccess = true;
if(pHttpTestEventLoop)
pHttpTestEventLoop->quit();
replyTest->deleteLater();
replyTest = nullptr;
}

Not sure if this is OK or not, I mean, setting the pointer to null before the pointed object is actually deleted when the scheduled delete event is taken care deep in some place out of your control.

[JonB]

@pablo-j-rogina
Although your observation is interesting, I don't think it is correct.

deleteLater() marks the data pointed to for later deletion, not the particular variable (replyTest), which it will never look at. That is my understanding. The accepted solution (whether that's a reliable guide or not is of course an issue :) ) at https://stackoverflow.com/a/6096158

There is nothing wrong if you call deleteLater() on your object and then set your pointer to nullptr. Qt framework will safely delete the object for you.

And @raven-worx wrote in https://forum.qt.io/topic/82195/how-to-set-a-pointer-to-an-object-to-nullptr-upon-calling-deletelater/9

So the pointer does, as the name says, only point to a memory address, but is not the object itself per se.

I am assuming that since the OP states it only happens with QNetworkProxy the issue has to lie in that code....

[Pablo J. Rogina]

@jonb said in Memory leak then using QNetworkProxy:

deleteLater() marks the data pointed to for later deletion, not the particular variable

Yes, that's why I don't like setting the pointer to null afterwards. Unless deleteLater() makes a copy of the address pointed at the time of method call, there'll be no way to delete anything then

[JonB]

@pablo-j-rogina
But it "does make a copy of the address", or mark the object pointed to for deletion, that's what it schedules for later deletion. Whether you change your variable or not is neither here nor there.

I think the point here is that either it's always safe to null your variable or it's never safe. Which is pretty fundamental to whether deleteLater() does or does not work reliably if you null. And I believe it's always safe. Not meaning to be rude, just saying.

No harm in the OP testing if he wishes, e.g. hook onto destroyed signal and only null there....

[Christian Ehrlicher]

@jonb said in Memory leak then using QNetworkProxy:

And I believe it's always safe

No need to only believe it - it's safe and I don't see where the assumption that it should not work comes from :)
Now to the real problem. First QNetworkAccessManager should not be created inside the loop but only once (see documentation). Then - how did you measure your memleak? I don't see anything problematic with valgrind nor heaptrack...

[denys]

@christian-ehrlicher said in Memory leak then using QNetworkProxy:

Now to the real problem. First QNetworkAccessManager should not be created inside the loop but only once (see documentation). Then - how did you measure your memleak? I don't see anything problematic with valgrind nor heaptrack...

really, if we create only one object of QNetworkAccessManager there is no memory leak. but documentation don't say that we can't create some instances of QNetworkAccessManager.
In my real app I have many threads - and each create personal instance of QNetworkAccessManager. So I need as many objects QNetworkAccessManager as I have threads.
About measuring memory leak - I just look at memory manager. Each time, after loop execution, memory used by program increasing by 1Mb.

[J.Hilk]

@denys said in Memory leak then using QNetworkProxy:

In my real app I have many threads - and each create personal instance of QNetworkAccessManager. So I need as many objects QNetworkAccessManager as I have threads.

do you really?
QNetworkAccessManager is asynchronous to be gin with and can handle 5(iirc) requests in parallel.

[Christian Ehrlicher]

@denys said in Memory leak then using QNetworkProxy:

I just look at memory manager

This is no indicator if there is a memory leak or not. It's not even an indication... use a correct tool.

[mrjj]

@j-hilk
6 i think :)
But i totally agree that this use case might not warrants using threads.

[denys]

@j-hilk said in Memory leak then using QNetworkProxy:

do you really?
QNetworkAccessManager is asynchronous to be gin with and can handle 5(iirc) requests in parallel.

QNetworkAccessManager Class
Note: All functions in this class are reentrant.
...a class is said to be reentrant if its member functions can be called safely from multiple threads, as long as each thread uses a different instance of the class

[denys]

@christian-ehrlicher said in Memory leak then using QNetworkProxy:

This is no indicator if there is a memory leak or not. It's not even an indication... use a correct tool.

Check it by Heob.
if I download from http (not https) or don't use proxy - there is 1 memory leak error - 665b for any count (>0) of loop pass
But when i try download from https and use proxy - there is same 1 memory leak error + 8 errors for each loop pass

results for loop with 5 passes:
https://drive.google.com/open?id=1YEo_BdAuQDSG4ZEfRTxyMup19STf6KS8

[Christian Ehrlicher]

@denys said in Memory leak then using QNetworkProxy:

8 errors for each loop pass

I can't find this in leaks(https,proxy) anywhere... can you please create html output

I can only see that there is a global static QGlobalNetworkProxy which is wrongly reported as leak (depending on the definition of a leak - QGlobalNetworkProxy it's only allocated once and not cleaned up on exit but that's what a Q_GLOBAL_STATIC is for.)

[denys]

This post is deleted!

[denys]

@christian-ehrlicher said in Memory leak then using QNetworkProxy:

I can't find this in leaks(https,proxy) anywhere... can you please create html output

https://drive.google.com/open?id=1_pBZlXd_LR5UJi5OCi-zH_VSgL-qgrgb

[Christian Ehrlicher]

Even your heob output prints some small leaks which may be real leaks I can't see anything here on linux with valgrind. I even added a debug output in the QHttpNetworkReply ctor and dtor to see if it is leaking but no way.
Your whole testcase looks fishy - esp. the re-creation of the QNAM inside the loop will for sure not be optimal and result in a comment in the bugreport about the wrong usage. I would suggest you to rewrite the testcase (to use threads) instead this imo wrong usage.