Smart pointers and avoid manual memory management when using QNetworkAccessManager

Question

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?

Qt docs say, that you should delete the reply, however not directly in the slot, but using `deleteLater()`. It looks like you do it properly in your code. — vahancho, Oct 02 '19 at 12:05
@vahancho could a `QSharedPointer` with a custom deleter be used then? (https://doc.qt.io/qt-5/qsharedpointer.html#QSharedPointer-2) — reckless, Oct 02 '19 at 12:07
It may be used, but I don't see a point for using a shared pointer. Who shares the pointer? In this case the scoped or unique pointer will be more appropriate, IMO. — vahancho, Oct 02 '19 at 12:10
but wouldn't the scope end before the `QNetworkReply::finished` is emitted? — reckless, Oct 02 '19 at 12:11
The scope would be the `send()` function. But anyways, I would't use smart pointers to manage the lifetime of pointers that aren't created by me. — vahancho, Oct 02 '19 at 12:15

score 0 · Accepted Answer · answered Oct 02 '19 at 15:36

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)

Smart pointers and avoid manual memory management when using QNetworkAccessManager

1 Answers1