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.