[CrystalSpace Homepage] #79: Race in csThreadManager::PushToQueue

#79: Race in csThreadManager::PushToQueue
---------------------------+------------------------------------------------
 Reporter:  ralphcampbell  |       Type:  defect
   Status:  new            |   Priority:  major 
Component:  content        |   Keywords:        
---------------------------+------------------------------------------------
 I found a race condition in csThreadManager::PushToQueue() when PlaneShift
 crashed.
 The problem is that the for loop is not protected by the
 waitingThreadsLock so the number of entries in the array can change while
 the loop is executing and can access invalid array entries if one is
 deleted.
 Since the loop only calls the condition variable NotifyAll() function,
 this can be called safely while holding the waitingThreadsLock lock.

 {{{
 Index: include/csutil/threadmanager.h
 ===================================================================
 --- include/csutil/threadmanager.h      (revision 39189)
 +++ include/csutil/threadmanager.h      (working copy)
 @@ -73,11 +73,8 @@
    {
      if(queueType == THREADED || queueType == THREADEDL)
      {
 -      {
 -        CS::Threading::MutexScopedLock lock(waitingThreadsLock);
 -        threadQueue->Enqueue(job);
 -      }
 -
 +      CS::Threading::MutexScopedLock lock(waitingThreadsLock);
 +      threadQueue->Enqueue(job);
        for(size_t i=0; i<waitingThreads.GetSize(); ++i)
        {
          waitingThreads[i]->NotifyAll();
 }}}

-- 
Ticket URL: <http://www.crystalspace3d.org/trac/homepage/ticket/79>
CrystalSpace Homepage <http://www.crystalspace3d.org/>
Homepage of the free open-source 3D SDK.
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar