Index: /trunk/src/VBox/Frontends/VirtualBox/src/globals/UIThreadPool.cpp
===================================================================
--- /trunk/src/VBox/Frontends/VirtualBox/src/globals/UIThreadPool.cpp	(revision 48907)
+++ /trunk/src/VBox/Frontends/VirtualBox/src/globals/UIThreadPool.cpp	(revision 48908)
@@ -1,7 +1,5 @@
 /* $Id$ */
 /** @file
- *
- * VBox frontends: Qt GUI ("VirtualBox"):
- * UIThreadPool class implementation
+ * VBox Qt GUI - UIThreadPool and UITask class implementation.
  */
 
@@ -32,6 +30,13 @@
 #include <VBox/sup.h>
 
-/* Worker-thread prototype.
- * Performs COM-related GUI tasks in separate thread. */
+/** @todo r=bird: this code was racy, inefficient and buggy in it's first
+ *        incarnation.  What is required from RTReqPool for it to replace the
+ *        internals here?  COM init/term hooks, sure. Does the threads need to
+ *        be QThreads? */
+
+
+/**
+ * COM capable worker thread for the UIThreadPool.
+ */
 class UIThreadWorker : public QThread
 {
@@ -48,7 +53,11 @@
     UIThreadWorker(UIThreadPool *pPool, int iIndex);
 
-    /* API: Busy stuff: */
-    bool isBusy() const;
-    void setBusy(bool fBusy);
+    int getIndex() const { return m_iIndex; }
+
+    /** Disables sigFinished.  For optimizing pool termination. */
+    void setNoFinishedSignal()
+    {
+        m_fNoFinishedSignal = true;
+    }
 
 private:
@@ -59,16 +68,19 @@
     /* Variables: General stuff: */
     UIThreadPool *m_pPool;
+
+    /** The index into UIThreadPool::m_workers. */
     int m_iIndex;
-
-    /* Variables: Busy stuff: */
-    bool m_fBusy;
-    mutable QMutex m_busyLocker;
+    /** Indicates whether sigFinished should be emitted or not. */
+    bool m_fNoFinishedSignal;
 };
 
 
-UIThreadPool::UIThreadPool(ulong uWorkerCount /* = 3*/, ulong uWorkerIdleTimeout /* = 5000*/)
-    : m_workers(uWorkerCount /* maximum worker count */)
-    , m_uIdleTimeout(uWorkerIdleTimeout) /* time for worker idle timeout */
+UIThreadPool::UIThreadPool(ulong cMaxWorkers/* = 3*/, ulong cMsWorkerIdleTimeout /* = 5000*/)
+    : m_workers(cMaxWorkers)
+    , m_cMsIdleTimeout(cMsWorkerIdleTimeout)
+    , m_cWorkers(0)
+    , m_cIdleWorkers(0)
     , m_fTerminating(false) /* termination status */
+    , m_everythingLocker(QMutex::Recursive)
 {
 }
@@ -76,48 +88,70 @@
 UIThreadPool::~UIThreadPool()
 {
-    /* Set termination status: */
-    setTerminating(true);
+    /* Set termination status and alert all idle worker threads: */
+    setTerminating();
 
     /* Cleanup all the workers: */
-    m_taskCondition.wakeAll();
-    for (int iWorkerIndex = 0; iWorkerIndex < m_workers.size(); ++iWorkerIndex)
-        cleanupWorker(iWorkerIndex);
+    m_everythingLocker.lock(); /* paranoia */
+    for (int idxWorker = 0; idxWorker < m_workers.size(); ++idxWorker)
+    {
+        UIThreadWorker *pWorker = m_workers[idxWorker];
+        m_workers[idxWorker] = NULL;
+
+        /* Clean up the worker, if there was one. */
+        if (pWorker)
+        {
+            m_cWorkers--;
+            m_everythingLocker.unlock();
+
+            pWorker->wait();
+
+            m_everythingLocker.lock();
+            delete pWorker;
+        }
+    }
+
+    m_everythingLocker.unlock();
 }
 
 void UIThreadPool::enqueueTask(UITask *pTask)
 {
+    Assert(!isTerminating());
+
     /* Prepare task: */
     connect(pTask, SIGNAL(sigComplete(UITask*)), this, SLOT(sltHandleTaskComplete(UITask*)), Qt::QueuedConnection);
 
-    /* Post task into queue: */
-    m_taskLocker.lock();
+    m_everythingLocker.lock();
+
+    /* Put the task onto the queue: */
     m_tasks.enqueue(pTask);
-    m_taskLocker.unlock();
-
-    /* Search for the first 'not yet created' worker or wake idle if exists: */
-    int iFirstNotYetCreatedWorkerIndex = -1;
-    for (int iWorkerIndex = 0; iWorkerIndex < m_workers.size(); ++iWorkerIndex)
-    {
-        /* Remember first 'not yet created' worker: */
-        if (iFirstNotYetCreatedWorkerIndex == -1 && !m_workers[iWorkerIndex])
-            iFirstNotYetCreatedWorkerIndex = iWorkerIndex;
-        /* But if worker 'not yet created' or 'busy' now, just skip it: */
-        if (!m_workers[iWorkerIndex] || m_workers[iWorkerIndex]->isBusy())
-            continue;
-        /* If we found idle worker, wake it up: */
+
+    /* Wake up an idle worker if we got one: */
+    if (m_cIdleWorkers > 0)
+    {
         m_taskCondition.wakeOne();
-        return;
-    }
-
-    /* Should we create new worker? */
-    if (iFirstNotYetCreatedWorkerIndex != -1)
-    {
-        /* Prepare worker: */
-        UIThreadWorker *pWorker = new UIThreadWorker(this, iFirstNotYetCreatedWorkerIndex);
-        connect(pWorker, SIGNAL(sigFinished(UIThreadWorker*)), this, SLOT(sltHandleWorkerFinished(UIThreadWorker*)), Qt::QueuedConnection);
-        m_workers[iFirstNotYetCreatedWorkerIndex] = pWorker;
-        /* And start it: */
-        pWorker->start();
-    }
+    }
+    /* No idle worker threads, should we create a new one? */
+    else if (m_cWorkers < m_workers.size())
+    {
+        /* Find free slot. */
+        int idxFirstUnused = m_workers.size();
+        while (idxFirstUnused-- > 0)
+            if (m_workers[idxFirstUnused] == NULL)
+            {
+                /* Prepare the new worker: */
+                UIThreadWorker *pWorker = new UIThreadWorker(this, idxFirstUnused);
+                connect(pWorker, SIGNAL(sigFinished(UIThreadWorker*)), this, 
+                        SLOT(sltHandleWorkerFinished(UIThreadWorker*)), Qt::QueuedConnection);
+                m_workers[idxFirstUnused] = pWorker;
+                m_cWorkers++;
+
+                /* And start it: */
+                pWorker->start();
+                break;
+            }
+    }
+    /* else: wait for some worker to complete whatever it's busy with and jump to it. */
+
+    m_everythingLocker.unlock();
 }
 
@@ -125,58 +159,78 @@
 {
     /* Acquire termination-flag: */
-    bool fTerminating = false;
-    {
-        QMutexLocker lock(&m_terminationLocker);
-        fTerminating = m_fTerminating;
-        Q_UNUSED(lock);
-    }
+    QMutexLocker lock(&m_everythingLocker);
+    bool fTerminating = m_fTerminating;
+    lock.unlock();
+
     return fTerminating;
 }
 
-void UIThreadPool::setTerminating(bool fTerminating)
-{
-    /* Assign termination-flag: */
-    QMutexLocker lock(&m_terminationLocker);
-    m_fTerminating = fTerminating;
-    Q_UNUSED(lock);
+void UIThreadPool::setTerminating()
+{
+    m_everythingLocker.lock();
+
+    /* Indicate that we're terminating: */
+    m_fTerminating = true;
+
+    /* Tell all threads to NOT queue any termination signals: */
+    for (int idxWorker = 0; idxWorker < m_workers.size(); ++idxWorker)
+    {
+        UIThreadWorker *pWorker = m_workers[idxWorker];
+        if (pWorker)
+            pWorker->setNoFinishedSignal();
+    }
+
+    /* Wake up all idle worker threads: */
+    m_taskCondition.wakeAll();
+
+    m_everythingLocker.unlock();
 }
 
 UITask* UIThreadPool::dequeueTask(UIThreadWorker *pWorker)
 {
-    /* Prepare task: */
-    UITask *pTask = 0;
-
-    /* Lock task locker: */
-    m_taskLocker.lock();
-
-    /* Try to dequeue task: */
-    if (!m_tasks.isEmpty())
-        pTask = m_tasks.dequeue();
-
-    /* Make sure we have a task or outdated: */
-    bool fOutdated = false;
-    while (!isTerminating() && !pTask && !fOutdated)
-    {
-        /* Mark thread as not busy: */
-        pWorker->setBusy(false);
-
-        /* Wait for <m_uIdleTimeout> milli-seconds for the next task,
-         * this issue will temporary unlock <m_taskLocker>: */
-        fOutdated = !m_taskCondition.wait(&m_taskLocker, m_uIdleTimeout);
-
-        /* Mark thread as busy again: */
-        pWorker->setBusy(true);
-
-        /* Try to dequeue task again: */
+    /*
+     * Dequeue a task, watching out for terminations. For opimal efficiency in
+     * enqueueTask() we keep count of idle threads.
+     *
+     * If the wait times out, we'll return NULL and terminate the thread.
+     */
+    m_everythingLocker.lock();
+
+    bool fIdleTimedOut = false;
+    while (!isTerminating())
+    {
+        Assert(m_workers[pWorker->getIndex()] == pWorker); /* paranoia */
+
+        /* Dequeue task if there is one: */
         if (!m_tasks.isEmpty())
-            pTask = m_tasks.dequeue();
-    }
-    Assert(isTerminating() || pTask || fOutdated);
-
-    /* Unlock task locker: */
-    m_taskLocker.unlock();
-
-    /* Return task: */
-    return pTask;
+        {
+            UITask *pTask = m_tasks.dequeue();
+            if (pTask)
+            {
+                m_everythingLocker.unlock();
+                return pTask;
+            }
+        }
+
+        /* If we timed out already, then quit the worker thread.  To prevent a
+           race between enqueueTask and the queue removal of the thread from
+           the workers vector, we remove it here already.  (This does not apply
+           to the termination scenario.) */
+        if (fIdleTimedOut)
+        {
+            m_workers[pWorker->getIndex()] = NULL;
+            m_cWorkers--;
+            break;
+        }
+
+        /* Wait for a task or timeout.*/
+        m_cIdleWorkers++;
+        fIdleTimedOut = !m_taskCondition.wait(&m_everythingLocker, m_cMsIdleTimeout);
+        m_cIdleWorkers--;
+    }
+
+    m_everythingLocker.unlock();
+
+    return NULL;
 }
 
@@ -193,24 +247,9 @@
 void UIThreadPool::sltHandleWorkerFinished(UIThreadWorker *pWorker)
 {
-    /* Skip on termination: */
-    if (isTerminating())
-        return;
-
-    /* Make sure that is one of our workers: */
-    int iIndexOfWorker = m_workers.indexOf(pWorker);
-    AssertReturnVoid(iIndexOfWorker != -1);
-
-    /* Cleanup worker: */
-    cleanupWorker(iIndexOfWorker);
-}
-
-void UIThreadPool::cleanupWorker(int iWorkerIndex)
-{
-    /* Cleanup worker if any: */
-    if (!m_workers[iWorkerIndex])
-        return;
-    m_workers[iWorkerIndex]->wait();
-    delete m_workers[iWorkerIndex];
-    m_workers[iWorkerIndex] = 0;
+    /* Wait for the thread to finish completely, then delete the thread
+       object.  We have already removed the thread from the workers vector.
+       Note! We don't want to use 'this' here, in case it's invalid. */
+    pWorker->wait();
+    delete pWorker;
 }
 
@@ -238,26 +277,6 @@
     : m_pPool(pPool)
     , m_iIndex(iIndex)
-    , m_fBusy(true)
-{
-}
-
-bool UIThreadWorker::isBusy() const
-{
-    /* Acquire busy-flag: */
-    bool fBusy = false;
-    {
-        QMutexLocker lock(&m_busyLocker);
-        fBusy = m_fBusy;
-        Q_UNUSED(lock);
-    }
-    return fBusy;
-}
-
-void UIThreadWorker::setBusy(bool fBusy)
-{
-    /* Assign busy-flag: */
-    QMutexLocker lock(&m_busyLocker);
-    m_fBusy = fBusy;
-    Q_UNUSED(lock);
+    , m_fNoFinishedSignal(false)
+{
 }
 
@@ -269,5 +288,5 @@
     COMBase::InitializeCOM(false);
 
-    /* Try to get task from thread-pool queue: */
+    /* Try get a task from the pool queue. */
     while (UITask *pTask = m_pPool->dequeueTask(this))
     {
@@ -276,4 +295,5 @@
         if (!m_pPool->isTerminating())
             pTask->start();
+        /** @todo else: Just leak the task? */
 //        LogRelFlow(("UIThreadWorker #%d: Task processed!\n", m_iIndex));
     }
@@ -282,6 +302,8 @@
     COMBase::CleanupCOM();
 
-    /* Notify listener: */
-    emit sigFinished(this);
+    /* Queue a signal to for the pool to do thread cleanup, unless the pool is
+       already terminating and doesn't need the signal. */
+    if (!m_fNoFinishedSignal)
+        emit sigFinished(this);
 
 //    LogRelFlow(("UIThreadWorker #%d: Finished!\n", m_iIndex));
Index: /trunk/src/VBox/Frontends/VirtualBox/src/globals/UIThreadPool.h
===================================================================
--- /trunk/src/VBox/Frontends/VirtualBox/src/globals/UIThreadPool.h	(revision 48907)
+++ /trunk/src/VBox/Frontends/VirtualBox/src/globals/UIThreadPool.h	(revision 48908)
@@ -1,6 +1,4 @@
 /** @file
- *
- * VBox frontends: Qt GUI ("VirtualBox"):
- * UIThreadPool class declaration
+ * VBox Qt GUI - UIThreadPool and UITask class declaration.
  */
 
@@ -17,6 +15,6 @@
  */
 
-#ifndef __UIThreadPool_h__
-#define __UIThreadPool_h__
+#ifndef ___UIThreadPool_h___
+#define ___UIThreadPool_h___
 
 /* Qt includes: */
@@ -46,5 +44,5 @@
 
     /* Constructor/destructor: */
-    UIThreadPool(ulong uWorkerCount = 3, ulong uWorkerIdleTimeout = 5000);
+    UIThreadPool(ulong cMaxWorkers = 3, ulong cMsWorkerIdleTimeout = 5000);
     ~UIThreadPool();
 
@@ -54,10 +52,10 @@
     /* API: Termination stuff: */
     bool isTerminating() const;
-    void setTerminating(bool fTermintating);
+    void setTerminating();
 
 protected:
 
     /* Protected API: Worker-thread stuff: */
-    UITask* dequeueTask(UIThreadWorker *pWorker);
+    UITask *dequeueTask(UIThreadWorker *pWorker);
 
 private slots:
@@ -70,20 +68,36 @@
 
 private:
+    /** @name Worker thread related variables.
+     * @{ */
+    QVector<UIThreadWorker*> m_workers;
+    /** Milliseconds  */
+    const ulong m_cMsIdleTimeout;
+    /** The number of worker threads.
+     * @remarks We cannot use the vector size since it may contain NULL pointers. */
+    int m_cWorkers;
+    /** The number of idle worker threads. */
+    int m_cIdleWorkers;
+    /** Set by the destructor to indicate that all worker threads should
+     *  terminate ASAP. */
+    bool m_fTerminating;
+    /** @} */
 
-    /* Helper: Worker stuff: */
-    void cleanupWorker(int iWorkerIndex);
+    /** @name Task related variables
+     * @{ */
+    /** Queue (FIFO) of pending tasks. */
+    QQueue<UITask*> m_tasks;
+    /** Condition variable that gets signalled when queuing a new task and there are
+     * idle worker threads around.
+     *
+     * Idle threads sits in dequeueTask waiting for this. Thus on thermination
+     * setTerminating() will do a broadcast signal to wake up all workers (after
+     * setting m_fTerminating of course). */
+    QWaitCondition m_taskCondition;
+    /** @} */
 
-    /* Variables: Worker stuff: */
-    QVector<UIThreadWorker*> m_workers;
-    const ulong m_uIdleTimeout;
 
-    /* Variables: Task stuff: */
-    QQueue<UITask*> m_tasks;
-    QMutex m_taskLocker;
-    QWaitCondition m_taskCondition;
-
-    /* Variables: Termination stuff: */
-    bool m_fTerminating;
-    mutable QMutex m_terminationLocker;
+    /** This mutex is used with the m_taskCondition condition and protects m_tasks,
+     *  m_fTerminating and m_workers. */
+    mutable QMutex m_everythingLocker;
 
     /* Friends: */
@@ -122,3 +136,4 @@
 };
 
-#endif /* __UIThreadPool_h__ */
+#endif /* !___UIThreadPool_h___ */
+
