AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QFuture - add ability to move results from QFuture

Browse Source 
QFuture's original design pre-dates C++11 and its
introduction of move semantics. QFuture is documented
as requiring copy-constructible classes and uses copy
operations for results (which in Qt's universe in general
is relatively cheap, due to the use of COW/data sharing).
QFuture::result(), QFuture::results(), QFuture::resultAt()
return copies. Now that the year is 2020, it makes some
sense to add support for move semantics and, in particular,
move-only types, like std::unique_ptr (that cannot be
obtained from QFuture using result etc.). Taking a result
or results from a QFuture renders it invalid.  This patch
adds QFuture<T>::takeResults(), takeResult() and isValid().
'Taking' functions are 'enabled_if' for non-void types only
to improve the compiler's diagnostic (which would otherwise
spit some semi-articulate diagnostic).
As a bonus a bug was found in the pre-existing code (after
initially copy and pasted into the new function) - the one
where we incorrectly report ready results in (rather obscure)
filter mode.

Fixes: QTBUG-81941
Fixes: QTBUG-83182
Change-Id: I8ccdfc50aa310a3a79eef2cdc55f5ea210f889c3
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Timur Pocheptsov 2020-02-26 10:40:02 +01:00
parent 986cfe312e
commit 44ceb56455
8 changed files with 455 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/concurrent/qtconcurrentrunbase.h
View File

@ -48,6 +48,9 @@
#include <QtCore/qrunnable.h>
#include <QtCore/qthreadpool.h>
#include <type_traits>
#include <utility>
QT_BEGIN_NAMESPACE
@ -123,7 +126,11 @@ public:
}
#endif
this->reportResult(result);
if constexpr (std::is_move_constructible_v<T>)
this->reportAndMoveResult(std::move(result));
else if constexpr (std::is_copy_constructible_v<T>)
this->reportResult(result);
this->reportFinished();
}
T result;

15
src/corelib/thread/qfuture.h
View File

@ -47,6 +47,9 @@
#include <QtCore/qfuture_impl.h>
#include <type_traits>
#include <vector>
QT_REQUIRE_CONFIG(future);
QT_BEGIN_NAMESPACE
@ -57,6 +60,10 @@ class QFutureWatcher;
template <typename T>
class QFuture
{
static_assert (std::is_copy_constructible_v<T>
|| std::is_move_constructible_v<T>
|| std::is_same_v<T, void>,
"Type with copy or move constructors or type void is required");
public:
QFuture()
: d(QFutureInterface<T>::canceledResult())
@ -135,6 +142,14 @@ public:
template<typename U = T, typename = QtPrivate::EnableForNonVoid<U>>
QList<T> results() const { return d.results(); }
template<typename U = T, typename = QtPrivate::EnableForNonVoid<U>>
T takeResult() { return d.takeResult(); }
template<typename U = T, typename = QtPrivate::EnableForNonVoid<U>>
std::vector<T> takeResults() { return d.takeResults(); }
bool isValid() const { return d.isValid(); }
template<class Function>
using ResultType = typename QtPrivate::ResultTypeHelper<Function, T>::ResultType;

98
src/corelib/thread/qfuture.qdoc
View File

@ -37,14 +37,17 @@
QFuture allows threads to be synchronized against one or more results
which will be ready at a later point in time. The result can be of any type
that has a default constructor and a copy constructor. If a result is not
available at the time of calling the result(), resultAt(), or results()
functions, QFuture will wait until the result becomes available. You can
use the isResultReadyAt() function to determine if a result is ready or
not. For QFuture objects that report more than one result, the
resultCount() function returns the number of continuous results. This
means that it is always safe to iterate through the results from 0 to
resultCount().
that has default, copy and possibly move constructors. If
a result is not available at the time of calling the result(), resultAt(),
results(), takeResult(), or takeResults() functions, QFuture
will wait until the result becomes available. You can use the isResultReadyAt()
function to determine if a result is ready or not. For QFuture objects that
report more than one result, the resultCount() function returns the number
of continuous results. This means that it is always safe to iterate through
the results from 0 to resultCount(). takeResult() and takeResults()
invalidate a future and any subsequent attempt to access result or results
from the future leads to undefined behavior. isValid() tells you if
results can be accessed.
QFuture provides a \l{Java-style iterators}{Java-style iterator}
(QFutureIterator) and an \l{STL-style iterators}{STL-style iterator}
@ -227,7 +230,7 @@
number of results stored might be different from this value, due to gaps
in the result set. It is always safe to iterate through the results from 0
to resultCount().
\sa result(), resultAt(), results()
\sa result(), resultAt(), results(), takeResult(), takeResults()
*/
/*! \fn template <typename T> int QFuture<T>::progressValue() const
@ -273,7 +276,10 @@
available, this function will block and wait for the result to become
available. This is a convenience method for calling resultAt(0).
\sa resultAt(), results()
\note Calling result() leads to undefined behavior if isValid()
returns \c false for this QFuture.
\sa resultAt(), results(), takeResult(), takeResults()
*/
/*! \fn template <typename T> T QFuture<T>::resultAt(int index) const
@ -282,7 +288,10 @@
immediately available, this function will block and wait for the result to
become available.
\sa result(), results(), resultCount()
\note Calling resultAt() leads to undefined behavior if isValid()
returns \c false for this QFuture.
\sa result(), results(), takeResult(), takeResults(), resultCount()
*/
/*! \fn template <typename T> bool QFuture<T>::isResultReadyAt(int index) const
@ -290,7 +299,10 @@
Returns \c true if the result at \a index is immediately available; otherwise
returns \c false.
\sa resultAt(), resultCount()
\note Calling isResultReadyAt() leads to undefined behavior if isValid()
returns \c false for this QFuture.
\sa resultAt(), resultCount(), takeResult(), takeResults()
*/
/*! \fn template <typename T> QFuture<T>::operator T() const
@ -300,15 +312,69 @@
available. This is a convenience method for calling result() or
resultAt(0).
\sa result(), resultAt(), results()
\note Calling this function leads to undefined behavior if isValid()
returns \c false for this QFuture.
\sa result(), resultAt(), results(), takeResult(), takeResults(), isValid()
*/
/*! \fn template <typename T> QList<T> QFuture<T>::results() const
Returns all results from the future. If the results are not immediately
available, this function will block and wait for them to become available.
Returns all results from the future. If the results are not immediately available,
this function will block and wait for them to become available.
\sa result(), resultAt(), resultCount()
\note Calling results() leads to undefined behavior if isValid()
returns \c false for this QFuture.
\sa result(), resultAt(), takeResult(), takeResults(), resultCount(), isValid()
*/
/*! \fn template <typename T> std::vector<T> QFuture<T>::takeResults()
If isValid() returns \c false, calling this function leads to undefined behavior.
takeResults() takes all results from the QFuture object and invalidates it
(isValid() will return \c false for this future). If the results are
not immediately available, this function will block and wait for them to
become available. This function tries to use move semantics for the results
if available and falls back to copy construction if the type is not movable.
\note QFuture in general allows sharing the results between different QFuture
objects (and potentially between different threads). takeResults() was introduced
to make QFuture also work with move-only types (like std::unique_ptr), so it
assumes that only one thread can move the results out of the future, and only
once.
\sa takeResult(), result(), resultAt(), results(), resultCount(), isValid()
*/
/* \fn template <typename T> std::vector<T> QFuture<T>::takeResult()
Call this function only if isValid() returns \c true, otherwise
the behavior is undefined. This function takes the first result from
the QFuture object, for convenience when only one result is expected.
If there are any other results, they are discarded after taking the
first one (if such behavior is undesired, use takeResults() instead).
If the result is not immediately available, this function will block and
wait for the result to become available. The QFuture will try to use move
semantics if possible, and will fall back to copy construction if the type
is not movable. After the result was taken, isValid() will evaluate
as \c false.
\note QFuture in general allows sharing the results between different QFuture
objects (and potentially between different threads). takeResult() was introduced
to make QFuture also work with move-only types (like std::unique_ptr), so it
assumes that only one thread can move the results out of the future, and
do it only once.
\sa takeResults(), result(), results(), resultAt(), isValid()
*/
/* \fn template <typename T> std::vector<T> QFuture<T>::isValid() const
Returns true if a result or results can be accessed or taken from this
QFuture object. Returns false after the result was taken from the future.
\sa takeResults(), takeResult(), result(), results(), resultAt()
*/
/*! \fn template <typename T> QFuture<T>::const_iterator QFuture<T>::begin() const

19
src/corelib/thread/qfutureinterface.cpp
View File

@ -114,6 +114,7 @@ void QFutureInterfaceBase::cancel()
d->waitCondition.wakeAll();
d->pausedWaitCondition.wakeAll();
d->sendCallOut(QFutureCallOutEvent(QFutureCallOutEvent::Canceled));
d->isValid = false;
}
void QFutureInterfaceBase::setPaused(bool paused)
@ -191,6 +192,12 @@ bool QFutureInterfaceBase::isResultReadyAt(int index) const
return d->internal_isResultReadyAt(index);
}
bool QFutureInterfaceBase::isValid() const
{
const QMutexLocker lock(&d->m_mutex);
return d->isValid;
}
bool QFutureInterfaceBase::isRunningOrPending() const
{
return queryState(static_cast<State>(Running | Pending));
@ -263,9 +270,9 @@ void QFutureInterfaceBase::reportStarted()
QMutexLocker locker(&d->m_mutex);
if (d->state.loadRelaxed() & (Started|Canceled|Finished))
return;
d->setState(State(Started | Running));
d->sendCallOut(QFutureCallOutEvent(QFutureCallOutEvent::Started));
d->isValid = true;
}
void QFutureInterfaceBase::reportCanceled()
@ -473,6 +480,16 @@ bool QFutureInterfaceBase::derefT() const
return d->refCount.derefT();
}
void QFutureInterfaceBase::reset()
{
d->m_progressValue = 0;
d->m_progressMinimum = 0;
d->m_progressMaximum = 0;
d->setState(QFutureInterfaceBase::NoState);
d->progressTime.invalidate();
d->isValid = false;
}
QFutureInterfaceBasePrivate::QFutureInterfaceBasePrivate(QFutureInterfaceBase::State initialState)
: refCount(1), m_progressValue(0), m_progressMinimum(0), m_progressMaximum(0),
state(initialState),

86
src/corelib/thread/qfutureinterface.h
View File

@ -45,6 +45,8 @@
#include <QtCore/qexception.h>
#include <QtCore/qresultstore.h>
#include <utility>
#include <vector>
#include <mutex>
QT_REQUIRE_CONFIG(future);
@ -116,6 +118,7 @@ public:
bool isPaused() const;
bool isThrottled() const;
bool isResultReadyAt(int index) const;
bool isValid() const;
void cancel();
void setPaused(bool paused);
@ -139,6 +142,7 @@ public:
protected:
bool refT() const;
bool derefT() const;
void reset();
public:
#ifndef QFUTURE_TEST
@ -198,13 +202,22 @@ public:
inline QFuture<T> future(); // implemented in qfuture.h
inline void reportResult(const T *result, int index = -1);
inline void reportAndMoveResult(T &&result, int index = -1);
inline void reportResult(const T &result, int index = -1);
inline void reportResults(const QVector<T> &results, int beginIndex = -1, int count = -1);
inline void reportFinished(const T *result = nullptr);
inline void reportFinished(const T *result);
void reportFinished()
{
QFutureInterfaceBase::reportFinished();
QFutureInterfaceBase::runContinuation();
}
inline const T &resultReference(int index) const;
inline const T *resultPointer(int index) const;
inline QList<T> results();
T takeResult();
std::vector<T> takeResults();
};
template <typename T>
@ -220,13 +233,28 @@ inline void QFutureInterface<T>::reportResult(const T *result, int index)
if (store.filterMode()) {
const int resultCountBefore = store.count();
store.addResult<T>(index, result);
this->reportResultsReady(resultCountBefore, resultCountBefore + store.count());
this->reportResultsReady(resultCountBefore, store.count());
} else {
const int insertIndex = store.addResult<T>(index, result);
this->reportResultsReady(insertIndex, insertIndex + 1);
}
}
template<typename T>
void QFutureInterface<T>::reportAndMoveResult(T &&result, int index)
{
std::lock_guard<QMutex> locker{mutex()};
if (queryState(Canceled) || queryState(Finished))
return;
QtPrivate::ResultStoreBase &store = resultStoreBase();
const int oldResultCount = store.count();
const int insertIndex = store.moveResult(index, std::forward<T>(result));
if (!store.filterMode() || oldResultCount < store.count()) // Let's make sure it's not in pending results.
reportResultsReady(insertIndex, store.count());
}
template <typename T>
inline void QFutureInterface<T>::reportResult(const T &result, int index)
{
@ -258,8 +286,7 @@ inline void QFutureInterface<T>::reportFinished(const T *result)
{
if (result)
reportResult(result);
QFutureInterfaceBase::reportFinished();
QFutureInterfaceBase::runContinuation();
reportFinished();
}
template <typename T>
@ -283,6 +310,7 @@ inline QList<T> QFutureInterface<T>::results()
exceptionStore().throwPossibleException();
return QList<T>();
}
QFutureInterfaceBase::waitForResult(-1);
QList<T> res;
@ -297,6 +325,56 @@ inline QList<T> QFutureInterface<T>::results()
return res;
}
template<typename T>
T QFutureInterface<T>::takeResult()
{
if (isCanceled()) {
exceptionStore().throwPossibleException();
return {};
}
if (!isValid())
return {};
// Note: we wait for all, this is intentional,
// not to mess with other unready results.
waitForResult(-1);
const std::lock_guard<QMutex> locker{mutex()};
QtPrivate::ResultIteratorBase position = resultStoreBase().resultAt(0);
T ret(std::move_if_noexcept(position.value<T>()));
reset();
resultStoreBase().template clear<T>();
return ret;
}
template<typename T>
std::vector<T> QFutureInterface<T>::takeResults()
{
if (isCanceled()) {
exceptionStore().throwPossibleException();
return {};
}
if (!isValid())
return {};
waitForResult(-1);
std::vector<T> res;
res.reserve(resultCount());
const std::lock_guard<QMutex> locker{mutex()};
QtPrivate::ResultIteratorBase it = resultStoreBase().begin();
for (auto endIt = resultStoreBase().end(); it != endIt; ++it)
res.push_back(std::move_if_noexcept(it.value<T>()));
reset();
resultStoreBase().template clear<T>();
return res;
}
template <>
class QFutureInterface<void> : public QFutureInterfaceBase
{

1
src/corelib/thread/qfutureinterface_p.h
View File

@ -197,6 +197,7 @@ public:
QBasicMutex continuationMutex;
bool launchAsync = false;
bool isValid = false;
};
QT_END_NAMESPACE

29
src/corelib/thread/qresultstore.h
View File

@ -43,6 +43,8 @@
#include <QtCore/qmap.h>
#include <QtCore/qdebug.h>
#include <utility>
QT_REQUIRE_CONFIG(future);
QT_BEGIN_NAMESPACE
@ -97,6 +99,19 @@ public:
return *pointer<T>();
}
template<typename T>
T &value()
{
return *pointer<T>();
}
template <typename T>
T *pointer()
{
const T *p = qAsConst(*this).pointer<T>();
return const_cast<T *>(p);
}
template <typename T>
const T *pointer() const
{
@ -144,8 +159,14 @@ public:
{
if (result == nullptr)
return addResult(index, static_cast<void *>(nullptr));
else
return addResult(index, static_cast<void *>(new T(*result)));
return addResult(index, static_cast<void *>(new T(*result)));
}
template <typename T>
int moveResult(int index, T &&result)
{
return addResult(index, static_cast<void *>(new T(std::move_if_noexcept(result))));
}
template <typename T>
@ -159,8 +180,8 @@ public:
{
if (m_filterMode == true && results->count() != totalCount && 0 == results->count())
return addResults(index, nullptr, 0, totalCount);
else
return addResults(index, new QVector<T>(*results), results->count(), totalCount);
return addResults(index, new QVector<T>(*results), results->count(), totalCount);
}
int addCanceledResult(int index)

225
tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
View File

@ -37,8 +37,12 @@
#include <qthreadpool.h>
#include <qexception.h>
#include <qrandom.h>
#include <QtConcurrent/qtconcurrentrun.h>
#include <private/qfutureinterface_p.h>
#include <vector>
#include <memory>
// COM interface macro.
#if defined(Q_OS_WIN) && defined(interface)
# undef interface
@ -101,6 +105,23 @@ private slots:
void thenOnExceptionFuture();
void thenThrows();
#endif
void takeResults();
void takeResult();
void runAndTake();
void resultsReadyAt_data();
void resultsReadyAt();
private:
using size_type = std::vector<int>::size_type;
using UniquePtr = std::unique_ptr<int>;
static void testSingleResult(const UniquePtr &p);
static void testSingleResult(const std::vector<int> &v);
template<class T>
static void testSingleResult(const T &unknown);
template<class T>
static void testFutureTaken(QFuture<T> &noMoreFuture);
template<class T>
static void testTakeResults(QFuture<T> future, size_type resultCount);
};
void tst_QFuture::resultStore()
@ -1173,7 +1194,6 @@ void tst_QFuture::iterators()
void tst_QFuture::iteratorsThread()
{
const int expectedResultCount = 10;
const int delay = 10;
QFutureInterface<int> futureInterface;
// Create result producer thread. The results are
@ -2061,5 +2081,208 @@ void tst_QFuture::thenThrows()
}
#endif
void tst_QFuture::testSingleResult(const UniquePtr &p)
{
QVERIFY(p.get() != nullptr);
}
void tst_QFuture::testSingleResult(const std::vector<int> &v)
{
QVERIFY(!v.empty());
}
template<class T>
void tst_QFuture::testSingleResult(const T &unknown)
{
Q_UNUSED(unknown);
}
template<class T>
void tst_QFuture::testFutureTaken(QFuture<T> &noMoreFuture)
{
QCOMPARE(noMoreFuture.isValid(), false);
QCOMPARE(noMoreFuture.resultCount(), 0);
QCOMPARE(noMoreFuture.isStarted(), false);
QCOMPARE(noMoreFuture.isRunning(), false);
QCOMPARE(noMoreFuture.isPaused(), false);
QCOMPARE(noMoreFuture.isFinished(), false);
QCOMPARE(noMoreFuture.progressValue(), 0);
}
template<class T>
void tst_QFuture::testTakeResults(QFuture<T> future, size_type resultCount)
{
auto copy = future;
QVERIFY(future.isFinished());
QVERIFY(future.isValid());
QCOMPARE(size_type(future.resultCount()), resultCount);
QVERIFY(copy.isFinished());
QVERIFY(copy.isValid());
QCOMPARE(size_type(copy.resultCount()), resultCount);
auto vec = future.takeResults();
QCOMPARE(vec.size(), resultCount);
for (const auto &r : vec) {
testSingleResult(r);
if (QTest::currentTestFailed())
return;
}
testFutureTaken(future);
if (QTest::currentTestFailed())
return;
testFutureTaken(copy);
}
void tst_QFuture::takeResults()
{
// Test takeResults() for movable types (whether or not copyable).
// std::unique_ptr<int> supports only move semantics:
QFutureInterface<UniquePtr> moveIface;
moveIface.reportStarted();
// std::vector<int> supports both copy and move:
QFutureInterface<std::vector<int>> copyIface;
copyIface.reportStarted();
const int expectedCount = 10;
for (int i = 0; i < expectedCount; ++i) {
moveIface.reportAndMoveResult(UniquePtr{new int(0b101010)}, i);
copyIface.reportAndMoveResult(std::vector<int>{1,2,3,4,5}, i);
}
moveIface.reportFinished();
copyIface.reportFinished();
testTakeResults(moveIface.future(), size_type(expectedCount));
if (QTest::currentTestFailed())
return;
testTakeResults(copyIface.future(), size_type(expectedCount));
}
void tst_QFuture::takeResult()
{
QFutureInterface<UniquePtr> iface;
iface.reportStarted();
iface.reportAndMoveResult(UniquePtr{new int(0b101010)}, 0);
iface.reportFinished();
auto future = iface.future();
QVERIFY(future.isFinished());
QVERIFY(future.isValid());
QCOMPARE(future.resultCount(), 1);
auto result = future.takeResult();
testFutureTaken(future);
if (QTest::currentTestFailed())
return;
testSingleResult(result);
}
void tst_QFuture::runAndTake()
{
// Test if a 'moving' future can be used by
// QtConcurrent::run.
auto rabbit = [](){
// Let's wait a bit to give the test below some time
// to sync up with us with its watcher.
QThread::currentThread()->msleep(100);
return UniquePtr(new int(10));
};
QTestEventLoop loop;
QFutureWatcher<UniquePtr> watcha;
connect(&watcha, &QFutureWatcher<UniquePtr>::finished, [&loop](){
loop.exitLoop();
});
auto gotcha = QtConcurrent::run(rabbit);
watcha.setFuture(gotcha);
loop.enterLoopMSecs(500);
if (loop.timeout())
QSKIP("Failed to run the task, nothing to test");
gotcha = watcha.future();
testTakeResults(gotcha, size_type(1));
}
void tst_QFuture::resultsReadyAt_data()
{
QTest::addColumn<bool>("testMove");
QTest::addRow("reportResult") << false;
QTest::addRow("reportAndMoveResult") << true;
}
void tst_QFuture::resultsReadyAt()
{
QFETCH(const bool, testMove);
QFutureInterface<int> iface;
QFutureWatcher<int> watcher;
watcher.setFuture(iface.future());
QTestEventLoop eventProcessor;
connect(&watcher, &QFutureWatcher<int>::finished, &eventProcessor, &QTestEventLoop::exitLoop);
const int nExpectedResults = 4;
int reported = 0;
int taken = 0;
connect(&watcher, &QFutureWatcher<int>::resultsReadyAt,
[&iface, &reported, &taken](int begin, int end)
{
auto future = iface.future();
QVERIFY(end - begin > 0);
for (int i = begin; i < end; ++i, ++reported) {
QVERIFY(future.isResultReadyAt(i));
taken |= 1 << i;
}
});
auto report = [&iface, testMove](int index)
{
int dummyResult = 0b101010;
if (testMove)
iface.reportAndMoveResult(std::move(dummyResult), index);
else
iface.reportResult(&dummyResult, index);
};
const QSignalSpy readyCounter(&watcher, &QFutureWatcher<int>::resultsReadyAt);
QTimer::singleShot(0, [&iface, &report]{
// With filter mode == true, the result may go into the pending results.
// Reporting it as ready will allow an application to try and access the
// result, crashing on invalid (store.end()) iterator dereferenced.
iface.setFilterMode(true);
iface.reportStarted();
report(0);
report(1);
// This one - should not be reported (it goes into pending):
report(3);
// Let's close the 'gap' and make them all ready:
report(-1);
iface.reportFinished();
});
// Run event loop, QCoreApplication::postEvent is in use
// in QFutureInterface:
eventProcessor.enterLoopMSecs(2000);
QVERIFY(!eventProcessor.timeout());
if (QTest::currentTestFailed()) // Failed in our lambda observing 'ready at'
return;
QCOMPARE(reported, nExpectedResults);
QCOMPARE(nExpectedResults, iface.future().resultCount());
QCOMPARE(readyCounter.count(), 3);
QCOMPARE(taken, 0b1111);
}
QTEST_MAIN(tst_QFuture)
#include "tst_qfuture.moc"