Fix QThreadPool::maxThreadCount() usage

The docs claim that QThreadPool always creates at least one thread.
However, the user can (usually by mistake) request zero or a negative
number of threads.
The maxThreadCount() function is simply returning the value, that was
requested by the user.
Since it's a public API, it is used in several places in QtConcurrent,
where it is assumed that the value is always positive. This can lead
to a crash if the user sets zero as a maxThreadCount.

Update all such places with std::max(maxThreadCount(), 1).
Prefer this approach over changing the return value of
maxThreadCount(), because its behavior is documented and tested.

Amends 885eff053797d56f2e295558d0a71b030fbb1a69.

Manual conflict resolutions:
 - Guard the std::max with the () trick against Windows min/max
   defines.

Fixes: QTBUG-120335
Change-Id: Id3b2087cec7fbc7a2d42febca6586f2dacffe444
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 936e72d18075b79c8d29353618dfbd052ae59dae)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 41a605e0f2251db78cb852bb161dd0020c63c935)
(cherry picked from commit e63224138625cafa19665dc18efe03f90915ef85)
(cherry picked from commit ac50160478f0192c64dcf87976a2870a6ac914b5)
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
This commit is contained in:
Ivan Solovev 2023-12-22 15:06:10 +01:00 committed by Marc Mutz
parent 6406d7ace3
commit 0af71d5578
4 changed files with 25 additions and 3 deletions

View File

@ -103,7 +103,7 @@ namespace QtConcurrent {
*/
BlockSizeManager::BlockSizeManager(QThreadPool *pool, int iterationCount)
: maxBlockSize(iterationCount / (pool->maxThreadCount() * 2)),
: maxBlockSize(iterationCount / (std::max(pool->maxThreadCount(), 1) * 2)),
beforeUser(0), afterUser(0),
m_blockSize(1)
{ }

View File

@ -153,7 +153,7 @@ class ReduceKernel
public:
ReduceKernel(QThreadPool *pool, ReduceOptions _reduceOptions)
: reduceOptions(_reduceOptions), progress(0), resultsMapSize(0),
threadCount(pool->maxThreadCount())
threadCount(std::max(pool->maxThreadCount(), 1))
{ }
void runReduce(ReduceFunctor &reduce,

View File

@ -993,7 +993,7 @@ void QHostInfoLookupManager::rescheduleWithMutexHeld()
isAlreadyRunning).second,
scheduledLookups.end());
const int availableThreads = threadPool.maxThreadCount() - currentLookups.size();
const int availableThreads = (std::max)(threadPool.maxThreadCount(), 1) - currentLookups.size();
if (availableThreads > 0) {
int readyToStartCount = qMin(availableThreads, scheduledLookups.size());
auto it = scheduledLookups.begin();

View File

@ -209,6 +209,17 @@ void tst_QtConcurrentMap::map()
QCOMPARE(list, NonTemplateSequence({ 2, 4, 6 }));
}
// custom pool with invalid number of threads
{
QList<int> list;
list << 1 << 2 << 3;
QThreadPool pool;
pool.setMaxThreadCount(0); // explicitly set incorrect value
// This should not crash
QtConcurrent::map(&pool, list, MultiplyBy2InPlace()).waitForFinished();
QCOMPARE(list, QList<int>() << 2 << 4 << 6);
}
#if 0
// not allowed: map() with immutable sequences makes no sense
{
@ -1046,6 +1057,17 @@ void tst_QtConcurrentMap::mappedReducedThreadPool()
intCube, intSumReduce);
QCOMPARE(result, sumOfCubes);
}
{
// pool with invalid number of threads
QThreadPool pool;
pool.setMaxThreadCount(0); // explicitly set incorrect value
// This should not crash
NonTemplateSequence list { 1, 2, 3 };
auto future = QtConcurrent::mappedReduced(&pool, list, multiplyBy2, intSumReduce);
QCOMPARE(future.result(), 12);
}
}
void tst_QtConcurrentMap::mappedReducedWithMoveOnlyCallable()