4

I've built a QFuture based asynchronous networking facade in my application. Roughly it works like this:

namespace NetworkFacade {
    QByteArray syncGet(const QUrl& url) {
        QEventLoop l;
        QByteArray rc;

        get(url, [&](const QByteArray& ba) {
            rc = ba;
            l.quit();
        });

        l.exec();
        return rc;
    }

    void get(const QUrl& url, const std::function<void (const QByteArray&)>& handler) {
        QPointer<QNetworkAccessManager> m = new QNetworkAccessManager;

        QObject::connect(m, &QNetworkAccessManager::finished, [=, &m](QNetworkReply *r) {
            QByteArray ba;

            if (r && r -> error() == QNetworkReply::NoError)
                ba = r -> readAll();

            m.clear();

            if (handler)
                handler(ba);
        });
        m -> get(QNetworkRequest(url));
    }
}

I have a QTimer that triggers a call on the main thread that does the following (obviously simplified):

foreach(Request r, requests) {
    futures.push_back(get(r));
}

foreach(QFuture<SomeType> f, futures) {
    f.waitForFinished();
    [do stuff with f.result()]
}

My assumption was that waitForFinished() would block the main thread while background thread(s) executed my network requests. Instead I get a qFatal error:

ASSERT: "m_blockedRunLoopTimer == m_runLoopTimer" in file eventdispatchers/qeventdispatcher_cf.mm, line 237

In the stack trace I see my waitForFinished() on the main thread, but then instead of being blocked I see (read from bottom up):

com.myapp   0x0008b669 QEventDispatcherCoreFoundation::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 1753
com.myapp   0x000643d7 QIOSEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 823
com.myapp   0x0130e3c7 QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 119
com.myapp   0x0130e5fb QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) + 539
com.myapp   0x0003a550 NetworkFacade::syncGet(QUrl const&) + 208
com.myapp   0x00037ed1 QtConcurrent::StoredFunctorCall0<std::__1::shared_ptr<QuoteFacade::Quote>, QuoteFacade::closingQuote(QString const&, QDate const&)::$_0>::runFunctor() + 49
com.myapp   0x00038967 QtConcurrent::RunFunctionTask<std::__1::shared_ptr<QuoteFacade::Quote> >::run() + 87
com.myapp   0x00038abc non-virtual thunk to QtConcurrent::RunFunctionTask<std::__1::shared_ptr<QuoteFacade::Quote> >::run() + 28
com.myapp   0x010dc40f QThreadPoolPrivate::stealRunnable(QRunnable*) + 431
com.myapp   0x010d0c35 QFutureInterfaceBase::waitForFinished() + 165

So rather than waiting for the QFuture to get a value, my supposedly concurrent task is issued on the main thread. This causes the get() function I outlined above to get invoked, which listens for events on the QEventLoop. Meanwhile, the QTimer fires again and I get the assertion from above.

Am I doing something wrong, or is it perfectly valid that QtConcurrent::run can cause control to go back to the main thread?

=== Update 1

@peppe: The lambda being executed simply does an HTTP GET and generates parses the JSON response into a SomeType object. The result being accessed via the QFuture.

=== Update 2

Apparently this is by design. From qfutureinterface.cpp from Qt 5.4.0 lines 293-295:

// To avoid deadlocks and reduce the number of threads used, try to
// run the runnable in the current thread.
d->pool()->d_func()->stealRunnable(d->runnable);
Tim
  • 4,560
  • 2
  • 40
  • 64
  • Why are you using QtConcurrent for dispatching network requests? And can you please elaborate of what's going on inside the lambda passed to `run`? I've got a feeling there's something wrong in there. – peppe Feb 04 '15 at 23:27
  • I want a simple way to issue multiple network requests and only look at the result later, i.e. using a future. `QtConcurrent` seemed like a good fit for that. Updated the question with more details about the lambda. – Tim Feb 04 '15 at 23:37
  • But QNetworkAccessManager is totally async by nature. They provide async signals for notification of what's going on. What's the problem at issuing the requests and inspecting them "later"? – peppe Feb 04 '15 at 23:57
  • Also I specificially asked what was being done in the lambda because thread affinity of QObjects plays a role. – peppe Feb 04 '15 at 23:57
  • QNetworkAccessManager is totally async, but only via signals/slots which aren't the nicest way to work in all cases. My algorithm will compute various things, some needing network data later, others needing stuff from a DB. I wanted a simple way to represent 'stuff fetched for later use'. A QFuture seems like an excellent way to abstract where data came from and provide very convenient access to it when I want it later. – Tim Feb 05 '15 at 00:09
  • Updated with full code showing how the network request is made. – Tim Feb 05 '15 at 00:11

1 Answers1

6

Apparently this is by design. From qfutureinterface.cpp from Qt 5.4.0 lines 293-295:

// To avoid deadlocks and reduce the number of threads used, try to
// run the runnable in the current thread.
d->pool()->d_func()->stealRunnable(d->runnable);

QtConcurrent::run() returns a QFuture which is implemented using a QFutureInterface. QFutureInterface contains that code in both waitForFinished() and waitForResult().

stealRunnable is an undocumented private method of QThreadPool. It is described thusly in headerdoc:

/*!
    \internal
    Searches for \a runnable in the queue, removes it from the queue and
    runs it if found. This function does not return until the runnable
    has completed.
*/

So what we wind up with is, if the QRunnable created internally by QtConcurrent::run() hasn't been dequeued from whatever QThreadPool it has been assigned to, then calling waitForFinished or waitForResult will cause it to run on the current thread (i.e. not concurrently.)

That means code like this (and what I did in the question) might fail in mysterious ways:

foreach (FuncPossiblyTriggeringQEvents fn, tasks) {
    futures.push_back(QtConcurrent::run(fn));
}

foreach (QFuture<> f, futures) {
    f.waitForFinished(); // Some of my tasks will run in this thread, not concurrently.
}

I got my design (getting a future out of a QNetowrkAccessManager get) working by using std::future and std::async.

Tim
  • 4,560
  • 2
  • 40
  • 64
  • 2
    This bit me -- and seems pretty broken ... It sure would be nice if you could at least turn this behavior off. Its particularly non intuitive when you explicitly specify the thread pool you want the function to run on -- but instead it runs on the calling thread ... -- eg: QtConcurrent::run(&myThreadPool, fn) – Ben Aug 16 '16 at 03:56
  • 1
    Yes, it is annoying. I'm using `std::async` instead, which is much better provided you can use `c++ 11`. – Tim Aug 17 '16 at 15:10
  • Would you be willing to share a code snippet showing how you converted above example to use std::async? I looked a little at std::async after seeing your answer to this -- but I'm not sure it will work for my use case. I have the need to schedule asynchronous work to run on a particular long-lived thread ... (I'm hackily achieving this in my scenario right now by using a QThreadPool with max thread size set to 1 and infinite thread expiry date ...) If you happen to know a way to achieve this with std::async I'd love some pointers ... – Ben Aug 17 '16 at 18:16
  • `std::async` doesn't come with a `std::thread_pool` unfortunately. But if you're using a `QThreadPool` you can connect a signal from your runnable to know when it is complete and avoid using `QtConcurrent` entirely. – Tim Aug 17 '16 at 19:21
  • Good advice -- I had been desiring not to run MOC in this part of the code -- this use case occurred within a small class that is part of an external interface used by non-qt projects ... Perhaps I can refactor and solve with slots/signals ... QtConcurrent does everything I need as is though (except for this weird quirk within waitForFinished()) ... I might just keep using QtConcurrent but solve this specific problem with a lock - it appears (I think) that not calling waitForFinished() is sufficient to never trigger the stealRunnable() code path ...? – Ben Aug 17 '16 at 22:43
  • 3
    Almost after 6 years, I faced the same issue. It's a badly designed API. Because of this broken behavior (and possibly few other issues), my little app has deadlock. – Nawaz Jan 24 '21 at 16:59
  • Problem still persists in QT6. Now the function is called `stealAndRunRunnable`. – Iizuki Sep 25 '22 at 12:08