Skip to content

Commit

Permalink
Fix dispatching when app (or thread) terminated
Browse files Browse the repository at this point in the history
Make sure to **not** notify handlers if the captured thread doesn't exist anymore, which would potentially result in dispatching to the wrong thread (ie. nullptr == current thread). This also applies when the app is shutting down and the even loop is not anymore available. In both cases, we should not trigger any error and skip notifications.
  • Loading branch information
simonbrunel committed Feb 24, 2018
1 parent f794916 commit 313d388
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
25 changes: 24 additions & 1 deletion src/qtpromise/qpromise_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace QtPromisePrivate {

// https://stackoverflow.com/a/21653558
template <typename F>
static void qtpromise_defer(F&& f, QThread* thread = nullptr)
static void qtpromise_defer(F&& f, const QPointer<QThread>& thread)
{
struct Event : public QEvent
{
Expand All @@ -43,11 +43,34 @@ static void qtpromise_defer(F&& f, QThread* thread = nullptr)
FType m_f;
};

if (!thread || thread->isFinished()) {
// Make sure to not call `f` if the captured thread doesn't exist anymore,
// which would potentially result in dispatching to the wrong thread (ie.
// nullptr == current thread). Since the target thread is gone, it should
// be safe to simply skip that notification.
return;
}

QObject* target = QAbstractEventDispatcher::instance(thread);
if (!target && QCoreApplication::closingDown()) {
// When the app is shutting down, the even loop is not anymore available
// so we don't have any way to dispatch `f`. This case can happen when a
// promise is resolved after the app is requested to close, in which case
// we should not trigger any error and skip that notification.
return;
}

Q_ASSERT_X(target, "postMetaCall", "Target thread must have an event loop");
QCoreApplication::postEvent(target, new Event(std::forward<F>(f)));
}

template <typename F>
static void qtpromise_defer(F&& f)
{
Q_ASSERT(QThread::currentThread());
qtpromise_defer(std::forward<F>(f), QThread::currentThread());
}

template <typename T>
struct PromiseDeduce
{
Expand Down
12 changes: 6 additions & 6 deletions tests/auto/qtpromise/future/tst_future.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,9 @@ void tst_future::fail()
QString result;
auto input = QPromise<QString>::reject(MyException("bar"));
auto output = input.fail([](const MyException& e) {
return QtConcurrent::run([=]() {
return QString("foo%1").arg(e.error());
});
return QtConcurrent::run([](const QString& error) {
return QString("foo%1").arg(error);
}, e.error());
});

QCOMPARE(input.isRejected(), true);
Expand All @@ -282,9 +282,9 @@ void tst_future::fail_void()
QString result;
auto input = QPromise<void>::reject(MyException("bar"));
auto output = input.fail([&](const MyException& e) {
return QtConcurrent::run([&]() {
result = e.error();
});
return QtConcurrent::run([&](const QString& error) {
result = error;
}, e.error());
});

QCOMPARE(input.isRejected(), true);
Expand Down

0 comments on commit 313d388

Please sign in to comment.