Search code examples
c++qtsmart-pointerssignals-slotsqsharedpointer

Smart pointers and avoid manual memory management when using QNetworkAccessManager


I have the following class which calls some HTTP API request to a server:

class NetworkRequest : public QObject {

public:
    NetworkRequest(QNetworkAccessManager* netManager):
        m_netManager(netManager){}

    void send(QUrl const &url){
        QNetworkRequest request(url);
        auto reply = m_netManager->get(request);
        connect(reply, &QNetworkReply::finished, [this, reply](){
            // do some stuff with reply (like if url is redirected)
            if(isRedirected(reply)){
                // we have to delete the reply and send a new one
                QUrl newUrl;
                // somehow get the new url;
                reply->deleteLater();
                send(newUrl);
            }
            else{
                reply->setParent(this); // so that it will be deleted when an instance of this class is deleted
                emit completed(reply);
            }
        });
    }

signals:
    void completed(QNetworkReply* reply);

private:
    QNetworkAccessManager* m_netManager;
    bool isRedirected(QNetworkReply * reply){
        bool yes = false;

        /* process the process reply and determine yes is true or false
        ....
        **/
        return yes;
    }
};

I use the class in this way:

auto req = new NetworkRequest(nm);
req->send("http://someurl.com");
connect(req, &NetworkRequest::completed, randomClass, &RandomClass::slot);

// in RandomClass::slot I call NetworkRequest::deleteLater() when I finished with the network data

Now this clearly involves some manual memory management and I have to be careful to not forget deleting the raw pointers. I was wondering if the code above could be written using QSharedPointer (or even std::shared_ptr) and replacing:

        auto reply = m_netManager->get(request);

with:

        auto smartReply = QSharedPointer<QNetworkReply>(m_netManager->get(request));

then replace all instances of reply with smartReply.get() and then forget about manually deleting reply objects. However, it is unclear to me if the shared pointer will automatically delete the objects because between the time frame that I call send() and the signal QNetworkReply::finished, would the smart pointer know that the raw pointer is still in use? Also when I delete an instance of NetworkRequest will the shared pointer automatically delete the QNetworkReply it owns?


Solution

  • Ok so after some thinking I came up with a solution. The main problem I wanted to solve was to avoid deleting my QNetworkReply* object manually, instead I wanted it to be automatically destroyed when an instance of NetworkRequest is deleted. In order to achieve this, I used a std::unique_ptr as a private member of my NetworkRequest class so when the class is destroyed, the unique_ptr automatically cleans the object in memory. Furthermore, by default std::unique_ptr deletes the object it hold upon reassignment, so whenever I call the send function in my, I can assign a new object to the smart pointer and the previous object in memory will get deleted automatically. One thing to notice, was the Qt docs recommend that QNetworkReply* should be deleted using QObject::deleteLater() (it is not fully clear to me why this is the case), in order to do this one can just use a custom deleter. So in my code, I declared a private member as following:

    private:
        struct deleteLaterDeletor
        {
            void operator()(QObject *object) const
            {
                if(object) {
                    object->deleteLater();
                }
            }
        };
    
        using ReplyPointer = std::unique_ptr<QNetworkReply, deleteLaterDeletor>;
    
        ReplyPointer m_reply;
    

    Then in my send function:

            m_reply = ReplyPointer(mNetManager->get(netRequest));
    

    Obviously, in the signature of signal and slots I have to pass the raw pointer (m_reply.get()).

    (QSharedPointer can also be used instead of std::unique_ptr)