From 313d3882d75453a760304a636cec2b968087cb2c Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Sat, 24 Feb 2018 12:31:03 +0100 Subject: [PATCH] Fix dispatching when app (or thread) terminated 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. --- src/qtpromise/qpromise_p.h | 25 +++++++++++++++++++++- tests/auto/qtpromise/future/tst_future.cpp | 12 +++++------ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/qtpromise/qpromise_p.h b/src/qtpromise/qpromise_p.h index f2012a5..c97e63d 100644 --- a/src/qtpromise/qpromise_p.h +++ b/src/qtpromise/qpromise_p.h @@ -32,7 +32,7 @@ namespace QtPromisePrivate { // https://stackoverflow.com/a/21653558 template -static void qtpromise_defer(F&& f, QThread* thread = nullptr) +static void qtpromise_defer(F&& f, const QPointer& thread) { struct Event : public QEvent { @@ -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))); } +template +static void qtpromise_defer(F&& f) +{ + Q_ASSERT(QThread::currentThread()); + qtpromise_defer(std::forward(f), QThread::currentThread()); +} + template struct PromiseDeduce { diff --git a/tests/auto/qtpromise/future/tst_future.cpp b/tests/auto/qtpromise/future/tst_future.cpp index 072d287..0508ac8 100644 --- a/tests/auto/qtpromise/future/tst_future.cpp +++ b/tests/auto/qtpromise/future/tst_future.cpp @@ -261,9 +261,9 @@ void tst_future::fail() QString result; auto input = QPromise::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); @@ -282,9 +282,9 @@ void tst_future::fail_void() QString result; auto input = QPromise::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);