diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h index 6c7c511cc07..9296c63f0b5 100644 --- a/src/corelib/thread/qfutureinterface.h +++ b/src/corelib/thread/qfutureinterface.h @@ -270,11 +270,12 @@ inline void QFutureInterface::reportResult(const T *result, int index) if (store.filterMode()) { const int resultCountBefore = store.count(); - store.addResult(index, result); - this->reportResultsReady(resultCountBefore, store.count()); + if (store.addResult(index, result) != -1) + this->reportResultsReady(resultCountBefore, store.count()); } else { const int insertIndex = store.addResult(index, result); - this->reportResultsReady(insertIndex, insertIndex + 1); + if (insertIndex != -1) + this->reportResultsReady(insertIndex, insertIndex + 1); } } @@ -289,7 +290,8 @@ void QFutureInterface::reportAndMoveResult(T &&result, int index) const int oldResultCount = store.count(); const int insertIndex = store.moveResult(index, std::forward(result)); - if (!store.filterMode() || oldResultCount < store.count()) // Let's make sure it's not in pending results. + // Let's make sure it's not in pending results. + if (insertIndex != -1 && (!store.filterMode() || oldResultCount < store.count())) reportResultsReady(insertIndex, store.count()); } @@ -317,11 +319,12 @@ inline void QFutureInterface::reportResults(const QList &_results, int beg if (store.filterMode()) { const int resultCountBefore = store.count(); - store.addResults(beginIndex, &_results, count); - this->reportResultsReady(resultCountBefore, store.count()); + if (store.addResults(beginIndex, &_results, count) != -1) + this->reportResultsReady(resultCountBefore, store.count()); } else { const int insertIndex = store.addResults(beginIndex, &_results, count); - this->reportResultsReady(insertIndex, insertIndex + _results.count()); + if (insertIndex != -1) + this->reportResultsReady(insertIndex, insertIndex + _results.count()); } } diff --git a/src/corelib/thread/qpromise.qdoc b/src/corelib/thread/qpromise.qdoc index 9c7925da953..4b7b497b3f0 100644 --- a/src/corelib/thread/qpromise.qdoc +++ b/src/corelib/thread/qpromise.qdoc @@ -112,6 +112,9 @@ Adds \a result to the internal result collection at \a index position. If index is unspecified, \a result is added to the end of the collection. + \note addResult() rejects \a result if there's already another result in the + collection stored at the same index. + You can get a result at a specific index by calling QFuture::resultAt(). \note It is possible to specify an arbitrary index and request result at diff --git a/src/corelib/thread/qresultstore.cpp b/src/corelib/thread/qresultstore.cpp index b6af27dfa98..a239954dbe0 100644 --- a/src/corelib/thread/qresultstore.cpp +++ b/src/corelib/thread/qresultstore.cpp @@ -144,6 +144,11 @@ bool ResultIteratorBase::canIncrementVectorIndex() const return (m_vectorIndex + 1 < mapIterator.value().m_count); } +bool ResultIteratorBase::isValid() const +{ + return mapIterator.value().isValid(); +} + ResultStoreBase::ResultStoreBase() : insertIndex(0), resultCount(0), m_filterMode(false), filteredResults(0) { } @@ -196,6 +201,15 @@ int ResultStoreBase::insertResultItem(int index, ResultItem &resultItem) return storeIndex; } +bool ResultStoreBase::containsValidResultItem(int index) const +{ + // index might refer to either visible or pending result + const bool inPending = m_filterMode && index != -1 && index > insertIndex; + const auto &store = inPending ? pendingResults : m_results; + auto it = findResult(store, index); + return it != ResultIteratorBase(store.end()) && it.isValid(); +} + void ResultStoreBase::syncPendingResults() { // check if we can insert any of the pending results: diff --git a/src/corelib/thread/qresultstore.h b/src/corelib/thread/qresultstore.h index 5f8c7f150a6..0a1382fd796 100644 --- a/src/corelib/thread/qresultstore.h +++ b/src/corelib/thread/qresultstore.h @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2020 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -87,6 +87,8 @@ public: bool operator!=(const ResultIteratorBase &other) const; bool isVector() const; bool canIncrementVectorIndex() const; + bool isValid() const; + protected: QMap::const_iterator mapIterator; int m_vectorIndex; @@ -139,6 +141,7 @@ public: protected: int insertResultItem(int index, ResultItem &resultItem); void insertResultItemIfValid(int index, ResultItem &resultItem); + bool containsValidResultItem(int index) const; void syncPendingResults(); void syncResultCount(); int updateInsertIndex(int index, int _count); @@ -169,6 +172,9 @@ public: template int addResult(int index, const T *result) { + if (containsValidResultItem(index)) // reject if already present + return -1; + if (result == nullptr) return addResult(index, static_cast(nullptr)); @@ -178,18 +184,27 @@ public: template int moveResult(int index, T &&result) { + if (containsValidResultItem(index)) // reject if already present + return -1; + return addResult(index, static_cast(new T(std::move_if_noexcept(result)))); } template int addResults(int index, const QList *results) { + if (containsValidResultItem(index)) // reject if already present + return -1; + return addResults(index, new QList(*results), results->count(), results->count()); } template int addResults(int index, const QList *results, int totalCount) { + if (containsValidResultItem(index)) // reject if already present + return -1; + if (m_filterMode == true && results->count() != totalCount && 0 == results->count()) return addResults(index, nullptr, 0, totalCount); @@ -198,12 +213,18 @@ public: int addCanceledResult(int index) { + if (containsValidResultItem(index)) // reject if already present + return -1; + return addResult(index, static_cast(nullptr)); } template int addCanceledResults(int index, int _count) { + if (containsValidResultItem(index)) // reject if already present + return -1; + QList empty; return addResults(index, &empty, _count); } diff --git a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp index 23ff646fb7f..99f21445289 100644 --- a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp +++ b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp @@ -149,6 +149,11 @@ private slots: void signalConnect(); void waitForFinished(); + void rejectResultOverwrite_data(); + void rejectResultOverwrite(); + void rejectPendingResultOverwrite_data() { rejectResultOverwrite_data(); } + void rejectPendingResultOverwrite(); + private: using size_type = std::vector::size_type; @@ -3109,5 +3114,182 @@ void tst_QFuture::waitForFinished() QVERIFY(waitingThread->isFinished()); } +void tst_QFuture::rejectResultOverwrite_data() +{ + QTest::addColumn("filterMode"); + QTest::addColumn>("initResults"); + + QTest::addRow("filter-mode-on-1-result") << true << QList({ 456 }); + QTest::addRow("filter-mode-on-N-results") << true << QList({ 456, 789 }); + QTest::addRow("filter-mode-off-1-result") << false << QList({ 456 }); + QTest::addRow("filter-mode-off-N-results") << false << QList({ 456, 789 }); +} + +void tst_QFuture::rejectResultOverwrite() +{ + QFETCH(bool, filterMode); + QFETCH(QList, initResults); + + QFutureInterface iface; + iface.setFilterMode(filterMode); + auto f = iface.future(); + QFutureWatcher watcher; + watcher.setFuture(f); + + QTestEventLoop eventProcessor; + // control the loop by suspend + connect(&watcher, &QFutureWatcher::suspending, &eventProcessor, &QTestEventLoop::exitLoop); + // internal machinery always emits resultsReadyAt + QSignalSpy resultCounter(&watcher, &QFutureWatcher::resultsReadyAt); + + // init + if (initResults.size() == 1) + iface.reportResult(initResults[0]); + else + iface.reportResults(initResults); + QCOMPARE(f.resultCount(), initResults.size()); + QCOMPARE(f.resultAt(0), initResults[0]); + QCOMPARE(f.results(), initResults); + + QTimer::singleShot(50, [&f]() { + f.suspend(); // should exit the loop + }); + // Run event loop, QCoreApplication::postEvent is in use + // in QFutureInterface: + eventProcessor.enterLoopMSecs(2000); + QVERIFY(!eventProcessor.timeout()); + QCOMPARE(resultCounter.count(), 1); + f.resume(); + + // overwrite with lvalue + { + int result = -1; + const auto originalCount = f.resultCount(); + iface.reportResult(result, 0); + QCOMPARE(f.resultCount(), originalCount); + QCOMPARE(f.resultAt(0), initResults[0]); + } + // overwrite with rvalue + { + const auto originalCount = f.resultCount(); + iface.reportResult(-1, 0); + QCOMPARE(f.resultCount(), originalCount); + QCOMPARE(f.resultAt(0), initResults[0]); + } + // overwrite with array + { + const auto originalCount = f.resultCount(); + iface.reportResults(QList { -1, -2, -3 }, 0); + QCOMPARE(f.resultCount(), originalCount); + QCOMPARE(f.resultAt(0), initResults[0]); + } + + // special case: add result by different index, overlapping with the vector + if (initResults.size() > 1) { + const auto originalCount = f.resultCount(); + iface.reportResult(-1, 1); + QCOMPARE(f.resultCount(), originalCount); + QCOMPARE(f.resultAt(1), initResults[1]); + } + + QTimer::singleShot(50, [&f]() { + f.suspend(); // should exit the loop + }); + eventProcessor.enterLoopMSecs(2000); + QVERIFY(!eventProcessor.timeout()); + QCOMPARE(resultCounter.count(), 1); + f.resume(); + QCOMPARE(f.results(), initResults); +} + +void tst_QFuture::rejectPendingResultOverwrite() +{ + QFETCH(bool, filterMode); + QFETCH(QList, initResults); + + QFutureInterface iface; + iface.setFilterMode(filterMode); + auto f = iface.future(); + QFutureWatcher watcher; + watcher.setFuture(f); + + QTestEventLoop eventProcessor; + // control the loop by suspend + connect(&watcher, &QFutureWatcher::suspending, &eventProcessor, &QTestEventLoop::exitLoop); + // internal machinery always emits resultsReadyAt + QSignalSpy resultCounter(&watcher, &QFutureWatcher::resultsReadyAt); + + // init + if (initResults.size() == 1) + iface.reportResult(initResults[0], 1); + else + iface.reportResults(initResults, 1); + QCOMPARE(f.resultCount(), 0); // not visible yet + if (!filterMode) { + QCOMPARE(f.resultAt(1), initResults[0]); + QCOMPARE(f.results(), initResults); + + QTimer::singleShot(50, [&f]() { + f.suspend(); // should exit the loop + }); + // Run event loop, QCoreApplication::postEvent is in use + // in QFutureInterface: + eventProcessor.enterLoopMSecs(2000); + QVERIFY(!eventProcessor.timeout()); + QCOMPARE(resultCounter.count(), 1); + f.resume(); + } + + // overwrite with lvalue + { + int result = -1; + const auto originalCount = f.resultCount(); + iface.reportResult(result, 1); + QCOMPARE(f.resultCount(), originalCount); + if (!filterMode) + QCOMPARE(f.resultAt(1), initResults[0]); + } + // overwrite with rvalue + { + const auto originalCount = f.resultCount(); + iface.reportResult(-1, 1); + QCOMPARE(f.resultCount(), originalCount); + if (!filterMode) + QCOMPARE(f.resultAt(1), initResults[0]); + } + // overwrite with array + { + const auto originalCount = f.resultCount(); + iface.reportResults(QList { -1, -2 }, 1); + QCOMPARE(f.resultCount(), originalCount); + if (!filterMode) + QCOMPARE(f.resultAt(1), initResults[0]); + } + // special case: add result by different index, overlapping with the vector + if (initResults.size() > 1) { + const auto originalCount = f.resultCount(); + iface.reportResult(-1, 2); + QCOMPARE(f.resultCount(), originalCount); + if (!filterMode) + QCOMPARE(f.resultAt(2), initResults[1]); + } + + if (!filterMode) { + QTimer::singleShot(50, [&f]() { + f.suspend(); // should exit the loop + }); + eventProcessor.enterLoopMSecs(2000); + QVERIFY(!eventProcessor.timeout()); + QCOMPARE(resultCounter.count(), 1); + f.resume(); + } + + iface.reportResult(123, 0); // make results at 0 and 1 accessible + QCOMPARE(f.resultCount(), initResults.size() + 1); + QCOMPARE(f.resultAt(1), initResults[0]); + initResults.prepend(123); + QCOMPARE(f.results(), initResults); +} + QTEST_MAIN(tst_QFuture) #include "tst_qfuture.moc" diff --git a/tests/auto/corelib/thread/qpromise/tst_qpromise.cpp b/tests/auto/corelib/thread/qpromise/tst_qpromise.cpp index 2b8853ccd7c..62e3711d423 100644 --- a/tests/auto/corelib/thread/qpromise/tst_qpromise.cpp +++ b/tests/auto/corelib/thread/qpromise/tst_qpromise.cpp @@ -169,12 +169,12 @@ void tst_QPromise::addResult() auto f = promise.future(); // add as lvalue + int resultAt0 = 456; { - int result = 456; - promise.addResult(result); + promise.addResult(resultAt0); QCOMPARE(f.resultCount(), 1); - QCOMPARE(f.result(), result); - QCOMPARE(f.resultAt(0), result); + QCOMPARE(f.result(), resultAt0); + QCOMPARE(f.resultAt(0), resultAt0); } // add as rvalue { @@ -190,13 +190,20 @@ void tst_QPromise::addResult() QCOMPARE(f.resultCount(), 3); QCOMPARE(f.resultAt(2), result); } - // add at position and overwrite + // add as lvalue at position and overwrite { int result = -1; const auto originalCount = f.resultCount(); promise.addResult(result, 0); QCOMPARE(f.resultCount(), originalCount); - QCOMPARE(f.resultAt(0), result); + QCOMPARE(f.resultAt(0), resultAt0); // overwrite does not work + } + // add as rvalue at position and overwrite + { + const auto originalCount = f.resultCount(); + promise.addResult(-1, 0); + QCOMPARE(f.resultCount(), originalCount); + QCOMPARE(f.resultAt(0), resultAt0); // overwrite does not work } }