Delete QNetworkReply* crash app

ricardobocchi

Hello everyone,

I recently had a problem freeing memory from a QNetworkReply*, and I can't find the cause. My code is the following:

(this is all the code needed to reproduce the error)

Http::~Http()
{
    //delete this->_reply; // here crash
    //this->_reply->deleteLater(); // here crash
}

void Http::get(){

        QEventLoop eventLoop;
        QNetworkAccessManager mgr;

        QObject::connect(&mgr, SIGNAL(finished(QNetworkReply*)), &eventLoop, SLOT(quit()));

        QNetworkRequest req( QUrl( QString("http://ip.jsontest.com/") ) );
        this->_reply = mgr.get(req);

        eventLoop.exec(); 
        this->processResponse();
}

void Http::processResponse(){

    if (this->_reply->error() == QNetworkReply::NoError) {
        qDebug() << "Success " << QString(this->_reply->readAll());
    }
    else {
        qDebug() << "Failure" << this->_reply->errorString();
    }

  // to test destructor class this code was commented.
   delete this->_reply; // Only here not crash,
}

If I delete _reply into method processResponse work. But if I use destructor to delete, the app crash. I also tried to wrap the reply with QSharedPointer, also without success. I'm curious to know why this happens.

If anyone can help me I'd appreciate it.

jsulm

@ricardobocchi said in Delete QNetworkReply* crash app:

But if I use destructor to delete, the app crash

Maybe the _reply pointer is invalid if you're trying to delete it in destructor, or it was already deleted in processResponse?
You should always assign nullptr to the pointer after deleting and check the pointer before deleting (only delete if it is not nullptr).

JonB

@ricardobocchi
As @jsulm says. Assuming your this->_reply has not somehow been overwritten to point to garbage. Since QNetworkReply is a QObject you can always connect QObject::destroyed() signal to help you see where it may have been destroyed.

ChrisW67

This synchronous usage of QNetworkAccessManager and friends may be seen by Qt as similar to calling delete in a slot attached to finished() (Your reply is in the finished state). Calling deleteLater() works?

Christian Ehrlicher

@ricardobocchi said in Delete QNetworkReply* crash app:

If anyone can help me I'd appreciate it.

Apart from the really strange idea to use a QEventLoop - QNetworkReply is a QObject, created by QNetworkAccessManager (and therefore ANAM is the parent). And since you delete QNetworkAccessManager every time it will also cleanup it's children.

ricardobocchi

@Christian-Ehrlicher Qt about QNetworkAccessManager resources:

Note: After the request has finished, it is the responsibility of the user to delete the QNetworkReply object at an appropriate time. Do not directly delete it inside the slot connected to finished(). You can use the deleteLater() function.

https://doc.qt.io/qt-6/qnetworkaccessmanager.html#details

@ChrisW67 deleteLater() on destructor not work.

@jsulm It is a very small and simple code, for testing and learning. Attempts to remove the pointer were made only in the class destructor, all with error.

@JonB I'll try to subscribe to the destruct event to see if it's being "magically" removed somewhere.

ricardobocchi

@JonB said in Delete QNetworkReply* crash app:

Hi,

I added the event:

QObject::connect(reply, SIGNAL(destroyed()), this, SLOT(mydestroyed()));

And found that the QNetworkReply is being destroyed "magically" as soon as the processResponse method ends. This seems to contradict what is written in the documentation.

If I give a dumpObjectInfo to _reply which is a member of the Http class, after the processResponse method finishes I get an error that crash the app.

void main(){

    Http http;
    http.get();
    // here destroyed is logged
    qDebug() << "reply is open? " << http.reaply()->isOpen(); // say true
    http.reaply()->dumpObjectInfo(); // app crash
    return QString(*r);

}

Maybe the QEventLoop or QNetworkAccessManager is scheduling the object's release, but that's not clear.

Christian Ehrlicher

@ricardobocchi said in Delete QNetworkReply* crash app:

Note: After the request has finished, it is the responsibility of the user to delete the QNetworkReply object at an appropriate time. Do not directly delete it inside the slot connected to finished(). You can use the deleteLater() function.
https://doc.qt.io/qt-6/qnetworkaccessmanager.html#details

And I'm still correct - the owner of QNetworkReply is QNetworkAccessManager - simply check it out by calling QNetworkReply::parent() and compare it with your QNAM.
Since the reply is only deleted when QNAM gets deleted (which should not happen after every request - it should stay as long as the app is running to e.g. re-use existing connections etc. ) you should cleanup your QNetworkReplys when you don't need them anymore to avoid unneeded memory usage with deleteLater().

Maybe the QEventLoop or QNetworkAccessManager is scheduling the object's release, but that's not clear.

Making a async event synch in Qt is very bad practice...

ricardobocchi

@Christian-Ehrlicher That's right, thanks for the help!