fixed: Race condition between jobworker timeout and worker deletion, where a job could sneak in the queue and have no worker available for it. Thanks to elupus.

git-svn-id: https://xbmc.svn.sourceforge.net/svnroot/xbmc/trunk@31400 568bbfeb-2a22-0410-94d2-cc84cf5bfa90

2 files changed, 35 insertions, 16 deletions

diff --git a/xbmc/utils/JobManager.cpp b/xbmc/utils/JobManager.cpp
index 7a449d4651..f74a176015 100644
--- a/xbmc/utils/JobManager.cpp
+++ b/xbmc/utils/JobManager.cpp
@@ -223,33 +223,47 @@ void CJobManager::StartWorkers(CJob::PRIORITY priority)
   m_workers.push_back(new CJobWorker(this));
 }
 
+CJob *CJobManager::PopJob()
+{
+  CSingleLock lock(m_section);
+  for (int priority = CJob::PRIORITY_HIGH; priority >= CJob::PRIORITY_LOW; --priority)
+  {
+    if (m_jobQueue[priority].size() && m_processing.size() < GetMaxWorkers(CJob::PRIORITY(priority)))
+    {
+      CWorkItem job = m_jobQueue[priority].front();
+      m_jobQueue[priority].pop_front();
+      // add to the processing vector
+      m_processing.push_back(job);
+      job.m_job->m_callback = this;
+      return job.m_job;
+    }
+  }
+  return NULL;
+}
+
 CJob *CJobManager::GetNextJob(const CJobWorker *worker)
 {
   CSingleLock lock(m_section);
   while (m_running)
   {
     // grab a job off the queue if we have one
-    for (int priority = CJob::PRIORITY_HIGH; priority >= CJob::PRIORITY_LOW; --priority)
-    {
-      if (m_jobQueue[priority].size() && m_processing.size() < GetMaxWorkers(CJob::PRIORITY(priority)))
-      {
-        CWorkItem job = m_jobQueue[priority].front();
-        m_jobQueue[priority].pop_front();
-        // add to the processing vector
-        m_processing.push_back(job);
-        job.m_job->m_callback = this;
-        return job.m_job;
-      }
-    }
+    CJob *job = PopJob();
+    if (job)
+      return job;
     // no jobs are left - sleep for 30 seconds to allow new jobs to come in
     lock.Leave();
-    if (!m_jobEvent.WaitMSec(30000))
-    { // timeout - say goodbye to the thread
-      break;
-    }
+    bool newJob = m_jobEvent.WaitMSec(30000);
     lock.Enter();
+    if (!newJob)
+      break;
     m_jobEvent.Reset();
   }
+  // ensure no jobs have come in during the period after
+  // timeout and before we held the lock
+  CJob *job = PopJob();
+  if (job)
+    return job;
+  // have no jobs
   RemoveWorker(worker);
   return NULL;
 }
diff --git a/xbmc/utils/JobManager.h b/xbmc/utils/JobManager.h
index a92fa66c99..d73b70da88 100644
--- a/xbmc/utils/JobManager.h
+++ b/xbmc/utils/JobManager.h
@@ -253,6 +253,11 @@ private:
   CJobManager const& operator=(CJobManager const&);
   virtual ~CJobManager();
 
+  /*! \brief Pop a job off the job queue and add to the processing queue ready to process
+   \return the job to process, NULL if no jobs are available
+   */
+  CJob *PopJob();
+
   void StartWorkers(CJob::PRIORITY priority);
   void RemoveWorker(const CJobWorker *worker);
   unsigned int GetMaxWorkers(CJob::PRIORITY priority) const;