< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

RE: [reSIProcate] AbstractFifo bug?


getNext() is fine as is, my mistake.

Here's my proposed fix for getNext(int ms):

void*
AbstractFifo::getNext(int ms)
{
   Lock lock(mMutex); (void)lock;

   const UInt64 end(Timer::getTimeMs() + ms);

   // Wait while there are messages available
   while (mFifo.empty())
   {
      // bail if total wait time exceeds limit
      bool signaled = mCondition.wait(mMutex, end - Timer::getTimeMs());
      if (!signaled)
      {
        return 0;
      }
   }

   // Return the first message on the fifo.
   void* firstMessage = mFifo.front();
   mFifo.pop_front();
   assert(mSize != 0);
   mSize--;
   return firstMessage;
}


Quoting Micky Kaufmann <micky@xxxxxxxxxxx>:

However now I?m more confused:

If I understand correctly you?re not asking why there?s a while in getNext() but int your fixed getnext(int ms).

As I see it getNext(int ms) pops an element from the fifo if and only if such an element exists in the timeslot described by ?ms?.

So the change from ?if? to ?while? seems wrong because if mfifo is not empty we don?t have to wait for a signal.

If the fifo is not empty, we will not wait for the signal. Note that the mutex
is locked on entry. The condition variable unlocks the mutex, waits for a
signal then relocks the mutex.

However, if the fifo is empty, we have to wait for the condition or timeout and
then recheck the fifo for contents. This last bit was missing in the 'if'
implementation. Further, the condition timer gets a little more complicated
with correct use of 'while'.

Thanks for pointing out the bug!

david


P.S. There have been rumors of problems with Windows condition variables. Mot
sure what to make of them.