Problem receiving signals between QNetworkReply, QTimer::singleShot and qeventloop

[Desujoi]

I'm having rather peculiar problem with my Qt application. The application communicates with another application through REST API. Calls are made by functions that will either return the response or empty value if timeout happens.
Code

QEventLoop loop;
    QNetworkReply* reply = nam_.post(request, jsonBytes);
    connect(reply, SIGNAL(finished()), &loop, SLOT(quit()));
    QTimer::singleShot(TIMEOUT, &loop, SLOT(quit())); //timeout
    QTimer::singleShot(TIMEOUT, this, SLOT(sayHello())); //timeout
 
    DBG << "pv Loop begin";
    loop.exec();
    DBG << "pv Loop end";
    QByteArray response = reply->readAll();
    return bytesToVariantMap(response);

QEventLoop loop;
    QNetworkReply* reply = nam_.post(request, jsonBytes);
    connect(reply, SIGNAL(finished()), &loop, SLOT(quit()));
    QTimer::singleShot(TIMEOUT, &loop, SLOT(quit())); //timeout
    QTimer::singleShot(TIMEOUT, this, SLOT(sayHello())); //timeout

    DBG << "pv Loop begin";
    loop.exec();
    DBG << "pv Loop end";
    QByteArray response = reply->readAll();
    return bytesToVariantMap(response);

To copy to clipboard, switch view to plain text mode 

In single try this works. However when multiple threads are running and calls are made in rapid succession problems occur: loop.exec() never exits and sayHello() is never executed. It seems as if the whole event system becomes unpredictable. Oddly enough adding delay before loop.exec seems to reduce the problem.

QThread::msleep(300); //Without this delay, loop.exec might deadlock (WTF...)
loop.exec();

QThread::msleep(300); //Without this delay, loop.exec might deadlock (WTF...)
loop.exec();

To copy to clipboard, switch view to plain text mode 

I tried this in both Qt 5.4 and 5.5

[jefftee]

In my opinion, you should abandon trying to force a synchronous result (using QEventLoop) for the QNetwork* functions, which are all asynchronous. Use the QNetworkReply::finished() signal instead to QNetworkReply::readAll() the result.

If you need to abort requests that exceed some timeout, you can use QTimer and when the timer pops, use QNetworkReply::abort() or stop the timer if you receive the finished signal first, etc.

[Desujoi]

Update to the problem. I isolated the problem so that only qeventloop and singleshots are used. It seems to work flawlessly.

QEventLoop loop;
QTimer::singleShot(TIMEOUT, &loop, SLOT(quit())); //timeout
QTimer::singleShot(TIMEOUT, this, SLOT(sayHello())); //timeout     
DBG << "pv Loop begin";
loop.exec();
DBG << "pv Loop end";

QEventLoop loop;
QTimer::singleShot(TIMEOUT, &loop, SLOT(quit())); //timeout
QTimer::singleShot(TIMEOUT, this, SLOT(sayHello())); //timeout     
DBG << "pv Loop begin";
loop.exec();
DBG << "pv Loop end";

To copy to clipboard, switch view to plain text mode 

---

Added after 5 minutes:

you should abandon trying to force a synchronous result

There are numerous reasons why I chose this approach. Main reason being that synchronous calls are a lot cleaner and resemble local calls which makes the code more approachable. Synchronous calls are not the core problem and hope this conversation does not derail into sync vs async.

[anda_skoa]

This isn't sync vs async since this is async.

It is of course your choice to do it this way, but make sure you are aware of the consequences.
- Nested event loops can lead to situations that are similar to re-entrancy, e.g. a method that "blocks" being called again.
- You need to either make sure that the object cannot be deleted while it is executing the nested event loop or you cannot rely on it existing after the event loop exits

The two most common patterns to avoid these problems are
- truely blocking
- encapsulating the async work in a "job" object and only handle its final result asynchronously

Cheers,
_

[Desujoi]

I have discovered that qeventloop is not independent from the maineventloop as is discussed in this forum post: http://stackoverflow.com/questions/2...s-manually-res
It seems that creating new qeventloop can in fact block maineventloop from running. AFAIK this means signals and slots will cease to work.

[anda_skoa]

It seems that creating new qeventloop can in fact block maineventloop from running. AFAIK this means signals and slots will cease to work.

No.
Hence the aforementions consequences of using nested event loops.
Which is also what the stackoverflow post describes.

Also signal/slots within a single thread don't depend on events.

Cheers,
_

[Desujoi]

Okay, so I ended up moving the class to another thread in its constructor.

moveToThread(&thread_);
thread_.start(QThread::HighestPriority);

moveToThread(&thread_);
thread_.start(QThread::HighestPriority);

To copy to clipboard, switch view to plain text mode 

Now "sayHello" slot started responding. However, qeventloop still does not quit on timeout. I tried to make qeventloop member variable and then called its quit method in sayHello(). Nothing happened, the program still locks in loop.exec(). I can't wrap my head around this.

[jefftee]

Synchronous calls are not the core problem and hope this conversation does not derail into sync vs async.

That certainly wasn't my intent at all, I simply stated my opinion, not fact. It's your code, you can do whatever you want, but as @anda_skoa pointed out, using QEventLoop has its drawbacks and pitfalls.

Edit: And to be clear for future readers, using QEventLoop doesn't change the behavior of the QNetwork* async calls, it simply is a kludge many have used (me included) to block while waiting for asynchronous calls to complete.

[Desujoi]

I think I figured it out!
a) QEventLoop::exec() does not only wait. It let's the same thread perform signal-slot-handling anywhere else in the code.
b) In my case execution was changed to a slot triggered by QTimer.
c) This slot halted in Mutex lock.
d) Hence whole qevenloop got frozen.
Feel free to correct me if I'm wrong. This was definitely a little confusing .
In essence: QTimer executes slot ->passes mutex -> qeventloop called -> execution moved back to the slot by qtimer -> mutex unpassable.

[anda_skoa]

I think I figured it out!
a) QEventLoop::exec() does not only wait. It let's the same thread perform signal-slot-handling anywhere else in the code.

No, the event loop is not involved with signal-slot-handling.

c) This slot halted in Mutex lock.
d) Hence whole qevenloop got frozen.

That's the very job of a mutex.
Only one thread can have the mutex locked at any given time.
It would be useless if it didn't block any further attempt at locking.

In essence: QTimer executes slot ->passes mutex -> qeventloop called -> execution moved back to the slot by qtimer -> mutex unpassable.

Why on earth do you use a mutex in a single threaded application?

Cheers,
_

[Desujoi]

Why on earth do you use a mutex in a single threaded application?
_

The answer is pretty simple: I thought it was not single threaded. My idea at the time was that QTimer spawned a new thread for every slot call. It is not at all obvious.

[anda_skoa]

The answer is pretty simple: I thought it was not single threaded. My idea at the time was that QTimer spawned a new thread for every slot call. It is not at all obvious.

I don't see anything in the QTimer documentation that would suggest it is using a thread.

That would be pretty horrible, it would make even otherwise simple programs hard to get right, safe multithreading is not trivial.

Cheers,
_

[Desujoi]

I ended up using QtConcurrent::run and creating thread specific QNetworkAccessManager when needed. This solves the dead lock of multiple slots waiting for the same response. This is temporary solution and I might end up separating nam calls into separate thread and then use thread join to wait.

[anda_skoa]

I might end up separating nam calls into separate thread and then use thread join to wait.

A.k.a "true blocking" as outlined as one of the possible solutions in comment #4

Cheers,
_