From e63224138625cafa19665dc18efe03f90915ef85 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Fri, 22 Dec 2023 15:06:10 +0100 Subject: [PATCH] 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. Fixes: QTBUG-120335 Pick-to: 6.5 6.2 Change-Id: Id3b2087cec7fbc7a2d42febca6586f2dacffe444 Reviewed-by: Thiago Macieira (cherry picked from commit 936e72d18075b79c8d29353618dfbd052ae59dae) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 41a605e0f2251db78cb852bb161dd0020c63c935) --- src/concurrent/qtconcurrentiteratekernel.cpp | 2 +- src/concurrent/qtconcurrentreducekernel.h | 2 +- src/network/kernel/qhostinfo.cpp | 2 +- .../qtconcurrentmap/tst_qtconcurrentmap.cpp | 22 +++++++++++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/concurrent/qtconcurrentiteratekernel.cpp b/src/concurrent/qtconcurrentiteratekernel.cpp index 088c8384ced..00228f181cb 100644 --- a/src/concurrent/qtconcurrentiteratekernel.cpp +++ b/src/concurrent/qtconcurrentiteratekernel.cpp @@ -67,7 +67,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) { } diff --git a/src/concurrent/qtconcurrentreducekernel.h b/src/concurrent/qtconcurrentreducekernel.h index cb7d7910aad..c337a9192fc 100644 --- a/src/concurrent/qtconcurrentreducekernel.h +++ b/src/concurrent/qtconcurrentreducekernel.h @@ -117,7 +117,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, diff --git a/src/network/kernel/qhostinfo.cpp b/src/network/kernel/qhostinfo.cpp index e7df47da6f2..2c6443b0ea3 100644 --- a/src/network/kernel/qhostinfo.cpp +++ b/src/network/kernel/qhostinfo.cpp @@ -965,7 +965,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(); diff --git a/tests/auto/concurrent/qtconcurrentmap/tst_qtconcurrentmap.cpp b/tests/auto/concurrent/qtconcurrentmap/tst_qtconcurrentmap.cpp index 94248404da4..e66edb1b602 100644 --- a/tests/auto/concurrent/qtconcurrentmap/tst_qtconcurrentmap.cpp +++ b/tests/auto/concurrent/qtconcurrentmap/tst_qtconcurrentmap.cpp @@ -185,6 +185,17 @@ void tst_QtConcurrentMap::map() QCOMPARE(list, NonTemplateSequence({ 2, 4, 6 })); } + // custom pool with invalid number of threads + { + QList 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() << 2 << 4 << 6); + } + #if 0 // not allowed: map() with immutable sequences makes no sense { @@ -1075,6 +1086,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()