A bug about QThreadPoolPrivate::runnableReady ?

[julien_lion]

Recently I study QThread source code, in "void QThreadPoolThread::run()" function, we can find below codes:

void QThreadPoolThread::run()
{
    QMutexLocker locker(&manager->mutex);
    for(;;) {
        QRunnable *r = runnable;
        runnable = 0;

        do {
            if (r) {
                const bool autoDelete = r->autoDelete();


                // run the task
                locker.unlock();
#ifndef QT_NO_EXCEPTIONS
                try {
#endif
                    r->run();
#ifndef QT_NO_EXCEPTIONS
                } catch (...) {
                    qWarning("Qt Concurrent has caught an exception thrown from a worker thread.\n"
                             "This is not supported, exceptions thrown in worker threads must be\n"
                             "caught before control returns to Qt Concurrent.");
                    registerTheadInactive();
                    throw;
                }
#endif
                locker.relock();

                if (autoDelete && !--r->ref)
                    delete r;
            }

            // if too many threads are active, expire this thread
            if (manager->tooManyThreadsActive())
                break;

            r = !manager->queue.isEmpty() ? manager->queue.takeFirst().first : 0;
        } while (r != 0);

        if (manager->isExiting) {
            registerTheadInactive();
            break;
        }

        // if too many threads are active, expire this thread
        bool expired = manager->tooManyThreadsActive();
        if (!expired) {
            ++manager->waitingThreads;
            registerTheadInactive();
            // wait for work, exiting after the expiry timeout is reached
            expired = !manager->runnableReady.wait(locker.mutex(), manager->expiryTimeout);
            ++manager->activeThreads;
    
            if (expired)
                --manager->waitingThreads;
        }
        if (expired) {
            manager->expiredThreads.enqueue(this);
            registerTheadInactive();
            break;
        }
    }
}

This snippet means that if current tasks are finished, runnableReady will wait for some time(30 seconds by default) like below:

manager->runnableReady.wait(locker.mutex(), manager->expiryTimeout);

During this period, if QThreadPoolPrivate::enqueueTask is called, a new runnable task will be added to queue like below:

void QThreadPoolPrivate::enqueueTask(QRunnable *runnable, int priority)
{
    if (runnable->autoDelete())
        ++runnable->ref;
    // put it on the queue
    QList<QPair<QRunnable *, int> >::iterator at =
        qUpperBound(queue.begin(), queue.end(), priority);
    queue.insert(at, qMakePair(runnable, priority));
    runnableReady.wakeOne();
}

Then, runnableReady in QThreadPoolThread::run will be waked up. However, as we see, the member variable "runnable" in class QThreadPoolThread is NULL, so QThreadPoolThread::run will return, this is not what we want !

In my opinion, since "runnableReady" is waked up, the thread should fetch the new runnable task in queue and execute it. In this way, QThreadPool can reuse existing threads. Was I wrong? Please give some help.

[VRonin]

While there are a few guys here that can look at that, I think you are better of sending your question to the dev mailing list http://lists.qt-project.org/mailman/listinfo/development

[julien_lion]

@julien_lion
Sorry, I misunderstood the code, it has no bug. Close it now.