1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix QtConcurrent algorithms to work with temporary sequences

Browse Source 
QtConcurrent algorithms are making an internal copy of the passed
sequence, to make sure it won't be destroyed before the execution
is finished. However, they were using iterators of the originally
passed sequence. So, if the original sequence is deleted, QtConcurrent
algorithms would use invalid iterators to a deleted sequence. This
might work with Qt containers thanks to implicit-sharing, but with
other containers will lead to unexpected results. Fixed them to work
on the internal copy of the original sequence.

Change-Id: I1d68692ed9746223c85f51bb05977bc1443b681d
Reviewed-by: Andreas Buhr <andreas.buhr@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Karsten Heimrich <karsten.heimrich@qt.io>
This commit is contained in:
Sona Kurazyan 2020-09-24 10:48:33 +02:00
parent b12d6c6a8a
commit a6e1f67937
5 changed files with 127 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/concurrent/qtconcurrentfunctionwrappers.h
View File

@ -164,6 +164,13 @@ struct MapSequenceResultType<InputSequence<T>, MapFunctor>
#endif // QT_NO_TEMPLATE_TEMPLATE_PARAMETER
template<typename Sequence>
struct SequenceHolder
{
SequenceHolder(const Sequence &s) : sequence(s) { }
Sequence sequence;
};
} // namespace QtPrivate.

9
src/concurrent/qtconcurrentmapkernel.h
View File

@ -219,20 +219,19 @@ inline ThreadEngineStarter<T> startMapped(QThreadPool *pool, Iterator begin,
sequence we are working on.
*/
template <typename Sequence, typename Base, typename Functor>
struct SequenceHolder1 : public Base
struct SequenceHolder1 : private QtPrivate::SequenceHolder<Sequence>, public Base
{
SequenceHolder1(QThreadPool *pool, const Sequence &_sequence, Functor functor)
: Base(pool, _sequence.begin(), _sequence.end(), functor), sequence(_sequence)
: QtPrivate::SequenceHolder<Sequence>(_sequence),
Base(pool, this->sequence.cbegin(), this->sequence.cend(), functor)
{ }
Sequence sequence;
void finish() override
{
Base::finish();
// Clear the sequence to make sure all temporaries are destroyed
// before finished is signaled.
sequence = Sequence();
this->sequence = Sequence();
}
};

35
src/concurrent/qtconcurrentreducekernel.h
View File

@ -41,6 +41,7 @@
#define QTCONCURRENT_REDUCEKERNEL_H
#include <QtConcurrent/qtconcurrent_global.h>
#include <QtConcurrent/qtconcurrentfunctionwrappers.h>
#if !defined(QT_NO_CONCURRENT) || defined(Q_CLANG_QDOC)
@ -222,37 +223,29 @@ public:
};
template <typename Sequence, typename Base, typename Functor1, typename Functor2>
struct SequenceHolder2 : public Base
struct SequenceHolder2 : private QtPrivate::SequenceHolder<Sequence>, public Base
{
SequenceHolder2(QThreadPool *pool,
const Sequence &_sequence,
Functor1 functor1,
Functor2 functor2,
ReduceOptions reduceOptions)
: Base(pool, _sequence.begin(), _sequence.end(), functor1, functor2, reduceOptions),
sequence(_sequence)
SequenceHolder2(QThreadPool *pool, const Sequence &_sequence, Functor1 functor1,
Functor2 functor2, ReduceOptions reduceOptions)
: QtPrivate::SequenceHolder<Sequence>(_sequence),
Base(pool, this->sequence.cbegin(), this->sequence.cend(), functor1, functor2,
reduceOptions)
{ }
template <typename InitialValueType>
SequenceHolder2(QThreadPool *pool,
const Sequence &_sequence,
Functor1 functor1,
Functor2 functor2,
InitialValueType &&initialValue,
ReduceOptions reduceOptions)
: Base(pool, _sequence.begin(), _sequence.end(), functor1, functor2,
std::forward<InitialValueType>(initialValue), reduceOptions),
sequence(_sequence)
template<typename InitialValueType>
SequenceHolder2(QThreadPool *pool, const Sequence &_sequence, Functor1 functor1,
Functor2 functor2, InitialValueType &&initialValue, ReduceOptions reduceOptions)
: QtPrivate::SequenceHolder<Sequence>(_sequence),
Base(pool, this->sequence.cbegin(), this->sequence.cend(), functor1, functor2,
std::forward<InitialValueType>(initialValue), reduceOptions)
{ }
Sequence sequence;
void finish() override
{
Base::finish();
// Clear the sequence to make sure all temporaries are destroyed
// before finished is signaled.
sequence = Sequence();
this->sequence = Sequence();
}
};

51
tests/auto/concurrent/qtconcurrentfilter/tst_qtconcurrentfilter.cpp
View File

@ -221,6 +221,13 @@ void tst_QtConcurrentFilter::filtered()
CHECK_FAIL("member");
testFiltered(intList, intListEven, lambdaIsEven);
CHECK_FAIL("lambda");
// rvalue sequences
auto future = QtConcurrent::filtered(std::vector { 1, 2, 3, 4 }, keepEvenIntegers);
QCOMPARE(future.results(), QList<int>({ 2, 4 }));
auto result = QtConcurrent::blockingFiltered(std::vector { 1, 2, 3, 4 }, keepEvenIntegers);
QCOMPARE(result, std::vector<int>({ 2, 4 }));
}
template <typename SourceObject,
@ -274,6 +281,14 @@ void tst_QtConcurrentFilter::filteredThreadPool()
CHECK_FAIL("function");
testFilteredThreadPool(&pool, intList, intListEven, lambdaIsOdd);
CHECK_FAIL("lambda");
// rvalue sequences
auto future = QtConcurrent::filtered(&pool, std::vector { 1, 2, 3, 4 }, keepEvenIntegers);
QCOMPARE(future.results(), QList<int>({ 2, 4 }));
auto result =
QtConcurrent::blockingFiltered(&pool, std::vector { 1, 2, 3, 4 }, keepEvenIntegers);
QCOMPARE(result, std::vector<int>({ 2, 4 }));
}
template <typename SourceObject,
@ -409,6 +424,15 @@ void tst_QtConcurrentFilter::filteredReduced()
CHECK_FAIL("lambda-member");
testFilteredReduced(intList, intSum, lambdaIsEven, lambdaIntSumReduce);
CHECK_FAIL("lambda-lambda");
// rvalue sequences
auto future = QtConcurrent::filteredReduced(std::vector { 1, 2, 3, 4 }, keepEvenIntegers,
intSumReduce);
QCOMPARE(future, intSum);
auto result = QtConcurrent::blockingFilteredReduced(std::vector { 1, 2, 3, 4 },
keepEvenIntegers, intSumReduce);
QCOMPARE(result, intSum);
}
template <typename SourceObject,
@ -485,6 +509,15 @@ void tst_QtConcurrentFilter::filteredReducedThreadPool()
CHECK_FAIL("lambda-function");
testFilteredReducedThreadPool(&pool, intList, intSum, lambdaIsOdd, lambdaSumReduce);
CHECK_FAIL("lambda-lambda");
// rvalue sequences
auto future = QtConcurrent::filteredReduced(&pool, std::vector { 1, 2, 3, 4 }, keepOddIntegers,
intSumReduce);
QCOMPARE(future, intSum);
auto result = QtConcurrent::blockingFilteredReduced(&pool, std::vector { 1, 2, 3, 4 },
keepOddIntegers, intSumReduce);
QCOMPARE(result, intSum);
}
void tst_QtConcurrentFilter::filteredReducedDifferentType()
@ -680,6 +713,15 @@ void tst_QtConcurrentFilter::filteredReducedInitialValue()
testFilteredReducedInitialValue(intList, intSum, lambdaIsEven,
lambdaIntSumReduce, intInitial);
CHECK_FAIL("lambda-lambda");
// rvalue sequences
auto future = QtConcurrent::filteredReduced(std::vector { 1, 2, 3, 4 }, keepEvenIntegers,
intSumReduce, intInitial);
QCOMPARE(future, intSum);
auto result = QtConcurrent::blockingFilteredReduced(std::vector { 1, 2, 3, 4 },
keepEvenIntegers, intSumReduce, intInitial);
QCOMPARE(result, intSum);
}
template <typename SourceObject,
@ -768,6 +810,15 @@ void tst_QtConcurrentFilter::filteredReducedInitialValueThreadPool()
testFilteredReducedInitialValueThreadPool(&pool, intList, intSum, lambdaIsOdd,
lambdaSumReduce, intInitial);
CHECK_FAIL("lambda-lambda");
// rvalue sequences
auto future = QtConcurrent::filteredReduced(&pool, std::vector { 1, 2, 3, 4 }, keepOddIntegers,
intSumReduce, intInitial);
QCOMPARE(future, intSum);
auto result = QtConcurrent::blockingFilteredReduced(&pool, std::vector { 1, 2, 3, 4 },
keepOddIntegers, intSumReduce, intInitial);
QCOMPARE(result, intSum);
}
void tst_QtConcurrentFilter::filteredReducedDifferentTypeInitialValue()

51
tests/auto/concurrent/qtconcurrentmap/tst_qtconcurrentmap.cpp
View File

@ -447,6 +447,14 @@ void tst_QtConcurrentMap::mapped()
testMapped(stringList, intList, &QString::toInt);
CHECK_FAIL("member");
#endif
// rvalue sequences
auto future = QtConcurrent::mapped(std::vector { 1, 2, 3 }, multiplyBy2);
QCOMPARE(future.results(), QList<int>({ 2, 4, 6 }));
auto result =
QtConcurrent::blockingMapped<std::vector<int>>(std::vector { 1, 2, 3 }, multiplyBy2);
QCOMPARE(result, std::vector<int>({ 2, 4, 6 }));
}
static QSemaphore semaphore(1);
@ -531,6 +539,14 @@ void tst_QtConcurrentMap::mappedThreadPool()
CHECK_FAIL("function");
testMappedThreadPool(&pool, intList, intListMultipiedBy3, lambdaMultiplyBy3);
CHECK_FAIL("lambda");
// rvalue sequences
auto future = QtConcurrent::mapped(&pool, std::vector { 1, 2, 3 }, multiplyBy2);
QCOMPARE(future.results(), QList<int>({ 2, 4, 6 }));
auto result = QtConcurrent::blockingMapped<std::vector<int>>(&pool, std::vector { 1, 2, 3 },
multiplyBy2);
QCOMPARE(result, std::vector<int>({ 2, 4, 6 }));
}
int intSquare(int x)
@ -657,6 +673,14 @@ void tst_QtConcurrentMap::mappedReduced()
CHECK_FAIL("lambda-member");
testMappedReduced(intList, sumOfSquares, lambdaSquare, lambdaSumReduce);
CHECK_FAIL("lambda-lambda");
// rvalue sequences
auto future = QtConcurrent::mappedReduced(std::vector { 1, 2, 3 }, intSquare, intSumReduce);
QCOMPARE(future, sumOfSquares);
auto result =
QtConcurrent::blockingMappedReduced(std::vector { 1, 2, 3 }, intSquare, intSumReduce);
QCOMPARE(result, sumOfSquares);
}
template <typename SourceObject, typename ResultObject, typename MapObject, typename ReduceObject>
@ -743,6 +767,15 @@ void tst_QtConcurrentMap::mappedReducedThreadPool()
CHECK_FAIL("lambda-function");
testMappedReducedThreadPool(&pool, intList, sumOfCubes, lambdaCube, lambdaSumReduce);
CHECK_FAIL("lambda-lambda");
// rvalue sequences
auto future =
QtConcurrent::mappedReduced(&pool, std::vector { 1, 2, 3 }, intCube, intSumReduce);
QCOMPARE(future, sumOfCubes);
auto result = QtConcurrent::blockingMappedReduced(&pool, std::vector { 1, 2, 3 }, intCube,
intSumReduce);
QCOMPARE(result, sumOfCubes);
}
void tst_QtConcurrentMap::mappedReducedDifferentType()
@ -905,6 +938,15 @@ void tst_QtConcurrentMap::mappedReducedInitialValue()
CHECK_FAIL("lambda-member");
testMappedReducedInitialValue(intList, sumOfSquares, lambdaSquare, lambdaSumReduce, intInitial);
CHECK_FAIL("lambda-lambda");
// rvalue sequences
auto future = QtConcurrent::mappedReduced(std::vector { 1, 2, 3 }, intSquare, intSumReduce,
intInitial);
QCOMPARE(future, sumOfSquares);
auto result = QtConcurrent::blockingMappedReduced(std::vector { 1, 2, 3 }, intSquare,
intSumReduce, intInitial);
QCOMPARE(result, sumOfSquares);
}
template <typename SourceObject, typename ResultObject, typename InitialObject, typename MapObject, typename ReduceObject>
@ -990,6 +1032,15 @@ void tst_QtConcurrentMap::mappedReducedInitialValueThreadPool()
testMappedReducedInitialValueThreadPool(&pool, intList, sumOfCubes, lambdaCube,
lambdaSumReduce, intInitial);
CHECK_FAIL("lambda-lambda");
// rvalue sequences
auto future = QtConcurrent::mappedReduced(&pool, std::vector { 1, 2, 3 }, intCube, intSumReduce,
intInitial);
QCOMPARE(future, sumOfCubes);
auto result = QtConcurrent::blockingMappedReduced(&pool, std::vector { 1, 2, 3 }, intCube,
intSumReduce, intInitial);
QCOMPARE(result, sumOfCubes);
}
void tst_QtConcurrentMap::mappedReducedDifferentTypeInitialValue()