microsoft / AMBROSIA

Robust Distributed Programming Made Easy and Efficient
https://microsoft.github.io/AMBROSIA/
Other
419 stars 49 forks source link

Bug in ElasticCircularBuffer #113

Open zllai opened 4 years ago

zllai commented 4 years ago

Hi Ambrosia team,

I am recently studying the Ambrosia source code. I think I may find a bug in ElasticCircularBuffer implemented in Ambrosia/Ambrosia/CircularBuffers.cs, though it seems not causing any trouble for now.

IEnumerable<T> Iterate()
{
    foreach (CircularBuffer<T> buffer in buffers)
    {
        foreach (T item in buffer.Iterate())
        {
             yield return item;
        }
    }
}

The first foreach iterates from buffers.First to buffers.Last. But this is actually not the desired behavior. Because when enqueued, the ElasticCircularBuffer reuses buffers.First if buffers.First contains an empty CircularBuffer, making buffers.First not actually the head. I also didn't see buffers.First being updated anywhere.

I guess it didn't cause any trouble because in testcases, CircularBuffer is big enough to hold all pending messages, so that ElasticCircularBuffer always contains only one CircularBuffer.

badrishc commented 4 years ago

Hi @zllai - thanks for reporting the issue. I wonder, can you provide a simple repro (program) that uses ElasticCircularBuffer stand alone, and shows the issue. That will help us diagnose and fix the data structure. If you have a fix, a PR would be very welcome as well :).