QNetworkReply::deleteLater problem

[TorAn]

I think that there is a problem with deleting QNetworkReply in case of continious communication between the client and server. I have a QEvenLoop-based class that initiates connection to the server, processes reply and repeates this cycle indefinitely.

In "isFinished" slot connected to corresponding QNetworkAccessManager signal I am calling QNetworkReply->deleteLater(). After 5-10 cycles I am getting an error. Review of the stack indicates that even though isFinished is called by accessmanager, it still wants to write something in networkReply instance later on when the instance is already deleted.

My workaround is: instead of calling "deleteLater" in "isFinished" slot I store instance of QNetworkReply in the smart pointer. Therefore QNetworReply is released not in the current cycle of communication, but in "next" cycle. My class design looks like this:


class myClass: QThread
{
signals:
void sendSomeDataToServer();
slot:
void onSendSomeDataToServer();
...
boost::shared_ptr<QNetworkReply> _delayedRelease;
QNetworkAccessManager _http;
}

myClass::myClass()
{
...
connect(this, sendSomeDataToServer, this, OnSendSomeDataToServer, Qt::ConnectionType::Queued);
}

myClass::run()
{
OnsendSomeDataToServer();
exec();
}

myclass::OnsendSomeDataToServer()
{
...
_http::post();
}
myclass::isFinished(QNetworReply* reply)
{
...
// reply->deleteLater(); does not work

_delayedRelease.reset(reply); // this works, I guess because it allows for some time to actually finish communication.

emit sendSomeDataToServer(); // this signal is processed in myclass instance; it is queued.
}
This workaround does work, but I will appreciate any comments/suggestions. After all, deleteLater() is called where it is supposed to be called based on the documentation for QNetworAccessManager "isFinished" slot. If necessary I can publish the stack and provide any necessary info.

[wysota]

Your code is flawed. You are accessing the _http variable which lives in the main thread from within the worker thread which is forbidden. I'm not surprised deleteLater() causes a segfault in this situation. If all your thread does is control the QNAM, then get rid of the thread and do it in the main thread, otherwise read about how to use threads properly and correct your code.

[TorAn]

May be I misunderstood your comment, but _http is a member variable of "myClass" instance. Why would you say that it is accessed from "worker thread"? Usage of myClass is analogous to this:


void main()
{
myClass a;
std::cin >>i;
}
where a is an instance of event loop. Network signals are processed in the instance "a".
There is no live worker thread here in a sense that "a" exited from "run" with "exec", leaving even loop alive.

[wysota]

May be I misunderstood your comment, but _http is a member variable of "myClass" instance. Why would you say that it is accessed from "worker thread"?
You access it (or exactly its child, the QNetworkReply) from the run() method, right? It's the worker thred function and the _http variable "lives" in the thread that created the myClass instance.


There is no live worker thread here in a sense that "a" exited from "run" with "exec", leaving even loop alive.
I'm afraid I don't understand what you mean here, I'm just reading the source code and I see you are accessing the _http variable from within run() which is ran in a separate thread by QThread::start() that I guess you are calling at some point. Otherwise exec() wouldn't be called and you'd get no event loop.

[TorAn]

I see your point now, thanks. Indeed, _http member variable lives in the main thread when I call a.start(). Run method of the "a" instance uses _http once and then exits with "exec()", leaving event loop alive.

I will rearrange the code by the scheme below. Do you consider it to be a proper usage of event loop and QNAM?


class myClass : public QThread
{
public:
myClass ()
{
connect(this, SIGNAL(doComm()), this, SLOT(onDoComm()), Qt::Queued);
}
void run() { exec(); }
void startComm() {emit doComm();}
signals:
void doComm();
slots:
void onDoComm() {_http.post(...);};
void isFinished(QNetworReply* r) {r->deleteLater(); emit doComm();}
private:
QNAM _http;
};

main
{
myClass a;
a.start();
a.startComm();

int i;
std::cin >> i;
}

[wysota]

Run method of the "a" instance uses _http once and then exits with "exec()", leaving event loop alive.
No. exec() blocks until the event loop quits and only then the thread is terminated.


I will rearrange the code by the scheme below. Do you consider it to be a proper usage of event loop and QNAM?
No. In my opinion you don't need the thread at all. Just create the application object and run its event loop.