summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSona Kurazyan <sona.kurazyan@qt.io>2020-03-26 13:22:46 +0100
committerSona Kurazyan <sona.kurazyan@qt.io>2020-04-01 21:51:06 +0200
commit495f958b9ac5c45196e743fcba300fcfd25b90a3 (patch)
treeeb24af3b1fffb770328b11dfccdfc3c783e7ff76
parent7909de1bebe3bac32286513025fc00220cd29ec1 (diff)
Store QFuture exceptions as std::exception_ptr
Replaced the internal ExceptionHolder for storing QException* by std::exception_ptr. This will allow to report and store exceptions of types that are not derived from QException. Task-number: QTBUG-81588 Change-Id: I96be919d8289448b3e608310e51a16cebc586301 Reviewed-by: Vitaly Fanaskov <vitaly.fanaskov@qt.io> Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
-rw-r--r--src/concurrent/qtconcurrentthreadengine.cpp2
-rw-r--r--src/corelib/thread/qexception.cpp55
-rw-r--r--src/corelib/thread/qexception.h18
-rw-r--r--src/corelib/thread/qfuture_impl.h7
-rw-r--r--src/corelib/thread/qfutureinterface.cpp9
-rw-r--r--src/corelib/thread/qfutureinterface.h1
-rw-r--r--tests/auto/corelib/thread/qfuture/tst_qfuture.cpp102
7 files changed, 116 insertions, 78 deletions
diff --git a/src/concurrent/qtconcurrentthreadengine.cpp b/src/concurrent/qtconcurrentthreadengine.cpp
index 7f91a2ba68..3293c49507 100644
--- a/src/concurrent/qtconcurrentthreadengine.cpp
+++ b/src/concurrent/qtconcurrentthreadengine.cpp
@@ -324,7 +324,7 @@ void ThreadEngineBase::handleException(const QException &exception)
{
if (futureInterface)
futureInterface->reportException(exception);
- else
+ else if (!exceptionStore.hasException())
exceptionStore.setException(exception);
}
#endif
diff --git a/src/corelib/thread/qexception.cpp b/src/corelib/thread/qexception.cpp
index f9c63085b7..5c7a05c9c2 100644
--- a/src/corelib/thread/qexception.cpp
+++ b/src/corelib/thread/qexception.cpp
@@ -158,65 +158,38 @@ QUnhandledException *QUnhandledException::clone() const
namespace QtPrivate {
-class Base : public QSharedData
-{
-public:
- Base(QException *exception)
- : exception(exception), hasThrown(false) { }
- ~Base() { delete exception; }
-
- QException *exception;
- bool hasThrown;
-};
-
-ExceptionHolder::ExceptionHolder(QException *exception)
-: base(exception ? new Base(exception) : nullptr) {}
-
-ExceptionHolder::ExceptionHolder(const ExceptionHolder &other)
-: base(other.base)
-{}
-
-void ExceptionHolder::operator=(const ExceptionHolder &other)
-{
- base = other.base;
-}
-
-ExceptionHolder::~ExceptionHolder()
-{}
-
-QException *ExceptionHolder::exception() const
+void ExceptionStore::setException(const QException &e)
{
- if (!base)
- return nullptr;
- return base->exception;
+ Q_ASSERT(!hasException());
+ try {
+ e.raise();
+ } catch (...) {
+ exceptionHolder = std::current_exception();
+ }
}
-void ExceptionStore::setException(const QException &e)
+void ExceptionStore::setException(std::exception_ptr e)
{
- if (hasException() == false)
- exceptionHolder = ExceptionHolder(e.clone());
+ Q_ASSERT(!hasException());
+ exceptionHolder = e;
}
bool ExceptionStore::hasException() const
{
- return (exceptionHolder.exception() != nullptr);
+ return !!exceptionHolder;
}
-ExceptionHolder ExceptionStore::exception()
+std::exception_ptr ExceptionStore::exception() const
{
return exceptionHolder;
}
void ExceptionStore::throwPossibleException()
{
- if (hasException() ) {
- exceptionHolder.base->hasThrown = true;
- exceptionHolder.exception()->raise();
- }
+ if (hasException())
+ std::rethrow_exception(exceptionHolder);
}
-bool ExceptionStore::hasThrown() const { return exceptionHolder.base->hasThrown; }
-
} // namespace QtPrivate
#endif //Q_CLANG_QDOC
diff --git a/src/corelib/thread/qexception.h b/src/corelib/thread/qexception.h
index d33904c1f2..b117d90caf 100644
--- a/src/corelib/thread/qexception.h
+++ b/src/corelib/thread/qexception.h
@@ -84,27 +84,15 @@ public:
namespace QtPrivate {
-class Base;
-class Q_CORE_EXPORT ExceptionHolder
-{
-public:
- ExceptionHolder(QException *exception = nullptr);
- ExceptionHolder(const ExceptionHolder &other);
- void operator=(const ExceptionHolder &other); // ### Qt6: copy-assign operator shouldn't return void. Remove this method and the copy-ctor, they are unneeded.
- ~ExceptionHolder();
- QException *exception() const;
- QExplicitlySharedDataPointer<Base> base;
-};
-
class Q_CORE_EXPORT ExceptionStore
{
public:
void setException(const QException &e);
+ void setException(std::exception_ptr e);
bool hasException() const;
- ExceptionHolder exception();
+ std::exception_ptr exception() const;
void throwPossibleException();
- bool hasThrown() const;
- ExceptionHolder exceptionHolder;
+ std::exception_ptr exceptionHolder;
};
} // namespace QtPrivate
diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h
index 20ee7e4f33..903b3787e3 100644
--- a/src/corelib/thread/qfuture_impl.h
+++ b/src/corelib/thread/qfuture_impl.h
@@ -226,10 +226,8 @@ void Continuation<Function, ResultType, ParentResultType>::runFunction()
}
}
#ifndef QT_NO_EXCEPTIONS
- } catch (QException &e) {
- promise.reportException(e);
} catch (...) {
- promise.reportException(QUnhandledException());
+ promise.reportException(std::current_exception());
}
#endif
promise.reportFinished();
@@ -249,8 +247,7 @@ bool Continuation<Function, ResultType, ParentResultType>::execute()
// interrupt the continuation chain, so don't report anything yet.
if constexpr (!std::is_invocable_v<std::decay_t<Function>, QFuture<ParentResultType>>) {
promise.reportStarted();
- const QException *e = parentFuture.d.exceptionStore().exception().exception();
- promise.reportException(*e);
+ promise.reportException(parentFuture.d.exceptionStore().exception());
promise.reportFinished();
return false;
}
diff --git a/src/corelib/thread/qfutureinterface.cpp b/src/corelib/thread/qfutureinterface.cpp
index 9d00bc3271..92b2df7c15 100644
--- a/src/corelib/thread/qfutureinterface.cpp
+++ b/src/corelib/thread/qfutureinterface.cpp
@@ -283,6 +283,15 @@ void QFutureInterfaceBase::reportCanceled()
#ifndef QT_NO_EXCEPTIONS
void QFutureInterfaceBase::reportException(const QException &exception)
{
+ try {
+ exception.raise();
+ } catch (...) {
+ reportException(std::current_exception());
+ }
+}
+
+void QFutureInterfaceBase::reportException(std::exception_ptr exception)
+{
QMutexLocker locker(&d->m_mutex);
if (d->state.loadRelaxed() & (Canceled|Finished))
return;
diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h
index 1da06beb7d..c8298d0742 100644
--- a/src/corelib/thread/qfutureinterface.h
+++ b/src/corelib/thread/qfutureinterface.h
@@ -90,6 +90,7 @@ public:
void reportCanceled();
#ifndef QT_NO_EXCEPTIONS
void reportException(const QException &e);
+ void reportException(std::exception_ptr e);
#endif
void reportResultsReady(int beginIndex, int endIndex);
diff --git a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
index 7e314d63ba..4e20741055 100644
--- a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
+++ b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
@@ -1418,6 +1418,23 @@ QFuture<void> createDerivedExceptionFuture()
return f;
}
+struct TestException
+{
+};
+
+QFuture<int> createCustomExceptionFuture()
+{
+ QFutureInterface<int> i;
+ i.reportStarted();
+ QFuture<int> f = i.future();
+ int r = 0;
+ i.reportResult(r);
+ auto exception = std::make_exception_ptr(TestException());
+ i.reportException(exception);
+ i.reportFinished();
+ return f;
+}
+
void tst_QFuture::exceptions()
{
// test throwing from waitForFinished
@@ -1502,6 +1519,18 @@ void tst_QFuture::exceptions()
}
QVERIFY(caught);
}
+
+ // Custom exceptions
+ {
+ QFuture<int> f = createCustomExceptionFuture();
+ bool caught = false;
+ try {
+ f.result();
+ } catch (const TestException &) {
+ caught = true;
+ }
+ QVERIFY(caught);
+ }
}
class MyClass
@@ -2040,46 +2069,87 @@ void tst_QFuture::thenOnExceptionFuture()
}
}
+template<class Exception, bool hasTestMsg = false>
+QFuture<void> createExceptionContinuation(QtFuture::Launch policy = QtFuture::Launch::Sync)
+{
+ QFutureInterface<void> promise;
+
+ auto then = promise.future().then(policy, [] {
+ if constexpr (hasTestMsg)
+ throw Exception("TEST");
+ else
+ throw Exception();
+ });
+
+ promise.reportStarted();
+ promise.reportFinished();
+
+ return then;
+}
+
void tst_QFuture::thenThrows()
{
- // Continuation throws an exception
+ // Continuation throws a QException
{
- QFutureInterface<void> promise;
+ auto future = createExceptionContinuation<QException>();
- QFuture<void> then = promise.future().then([]() { throw QException(); });
+ bool caught = false;
+ try {
+ future.waitForFinished();
+ } catch (const QException &) {
+ caught = true;
+ }
+ QVERIFY(caught);
+ }
- promise.reportStarted();
- promise.reportFinished();
+ // Continuation throws an exception derived from QException
+ {
+ auto future = createExceptionContinuation<DerivedException>();
bool caught = false;
try {
- then.waitForFinished();
- } catch (QException &) {
+ future.waitForFinished();
+ } catch (const QException &) {
caught = true;
+ } catch (const std::exception &) {
+ QFAIL("The exception should be caught by the above catch block.");
}
+
QVERIFY(caught);
}
- // Same with QtFuture::Launch::Async
+ // Continuation throws std::exception
{
- QFutureInterface<void> promise;
+ auto future = createExceptionContinuation<std::runtime_error, true>();
- QFuture<void> then =
- promise.future().then(QtFuture::Launch::Async, []() { throw QException(); });
+ bool caught = false;
+ try {
+ future.waitForFinished();
+ } catch (const QException &) {
+ QFAIL("The exception should be caught by the below catch block.");
+ } catch (const std::exception &e) {
+ QCOMPARE(e.what(), "TEST");
+ caught = true;
+ }
- promise.reportStarted();
- promise.reportFinished();
+ QVERIFY(caught);
+ }
+
+ // Same with QtFuture::Launch::Async
+ {
+ auto future = createExceptionContinuation<QException>(QtFuture::Launch::Async);
bool caught = false;
try {
- then.waitForFinished();
- } catch (QException &) {
+ future.waitForFinished();
+ } catch (const QException &) {
caught = true;
}
QVERIFY(caught);
}
}
-#endif
+
+#endif // QT_NO_EXCEPTIONS
void tst_QFuture::testSingleResult(const UniquePtr &p)
{