[reSIProcate] AbstractFifo bug?

david Butcher david at purplecomm.com
Tue May 24 17:00:29 CDT 2005


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 at proxy.co.il>:

> 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.



More information about the resiprocate-devel mailing list