Details
-
Bug
-
Resolution: Done
-
P2: Important
-
5.15.2
-
None
-
-
885eff053797d56f2e295558d0a71b030fbb1a69 (qt/qtbase/dev) cd3ce03705e32f2d1bd566fc6ad9ae215cff56b7 (qt/qtbase/6.1) c0d310d9969111713ec705db1c6c4833c92df468 (qt/qtbase/6.0)
Description
I have a QThreadPool that I set maxThreadCount to QThread::idealThreadCount() - 1. I didn't bother clamping the value to > 0 since the QThreadPool docs says "Note: The thread pool will always use at least 1 thread, even if maxThreadCount limit is zero or negative."
This might have been true sometime in the past but as the code is written currently it's no longer the case.
What happened was that my code was run on a pc with a single core which resulted in the max thread count being set to 0. I started one runner and then 30 seconds passed, which is the default expiry time for the threads, and then I tried starting some more runners which then never got started. I had my suspicion that the 0 max count was the problem but as it was threading I wanted to be sure, so after a lot of debugging I figured out that the issue is with the code in QThreadPoolPrivate::tryStart:
bool QThreadPoolPrivate::tryStart(QRunnable *task) { Q_ASSERT(task != nullptr); if (allThreads.isEmpty()) { // always create at least one thread startThread(task); return true; } // can't do anything if we're over the limit if (activeThreadCount() >= maxThreadCount) return false; if (waitingThreads.count() > 0) { // recycle an available thread enqueueTask(task); waitingThreads.takeFirst()->runnableReady.wakeOne(); return true; } if (!expiredThreads.isEmpty()) { // restart an expired thread QThreadPoolThread *thread = expiredThreads.dequeue(); Q_ASSERT(thread->runnable == nullptr); ++activeThreads; thread->runnable = task; thread->start(threadPriority); return true; } // start a new thread startThread(task); return true; }
The first if would evaluate to false since I had one expired thread but the second if would always evaluate to true since 0 >= 0, meaning the expired thread would never be recovered, which happens further down in this function.
The solution could be to just std::max(1, maxThreadCount) as I now did in my code, or fix the logic above. Or just change the docs
Attachments
For Gerrit Dashboard: QTBUG-93007 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
344747,4 | QThreadPool: obey the docs that say we always use at least 1 thread | dev | qt/qtbase | Status: MERGED | +2 | 0 |
345264,2 | QThreadPool: obey the docs that say we always use at least 1 thread | 6.0 | qt/qtbase | Status: MERGED | +2 | 0 |
345265,2 | QThreadPool: obey the docs that say we always use at least 1 thread | 6.1 | qt/qtbase | Status: MERGED | +2 | 0 |