From 2750ec8ef98211a6d96077d0dc7c25848db5dfc1 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Sun, 23 Jan 2022 17:43:30 -0800 Subject: [PATCH] [util] Fix bip_buffer concurrency bug If a bip_buffer's A buffer is in the middle of being appended to, but that append has not yet been committed, and all committed A data has been read, the buffer would get into a bad state where m_start_r pointed to the end of the previous A buffer, but that data is no longer connected to either A or B. So now we make sure to check m_size_r before considering A "done". --- src/libraries/util/bip_buffer.cpp | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/libraries/util/bip_buffer.cpp b/src/libraries/util/bip_buffer.cpp index 5133252..548b8f8 100644 --- a/src/libraries/util/bip_buffer.cpp +++ b/src/libraries/util/bip_buffer.cpp @@ -71,16 +71,16 @@ void bip_buffer::commit(size_t size) scoped_lock lock {m_lock}; assert(size <= m_size_r && "Tried to commit more than reserved"); - - if (m_start_r == m_start_a + m_size_a) { - // We were adding to A - m_size_a += size; - } else { - // We were adding to B - assert(m_start_r == m_start_b + m_size_b && "Bad m_start_r!"); - m_size_b += size; + if (m_size_r) { + if (m_start_r == m_start_a + m_size_a) { + // We were adding to A + m_size_a += size; + } else { + // We were adding to B + assert(m_start_r == m_start_b + m_size_b && "Bad m_start_r!"); + m_size_b += size; + } } - m_start_r = m_size_r = 0; } @@ -98,9 +98,16 @@ void bip_buffer::consume(size_t size) assert(size <= m_size_a && "Consumed more bytes than exist in A"); if (size >= m_size_a) { - m_size_a = m_size_b; - m_start_a = m_start_b; - m_size_b = m_start_b = 0; + if (m_size_r && m_start_r == m_start_a + m_size_a) { + // A is still being appended to + m_start_a = m_start_r; + m_size_a = 0; + } else { + // A is done, B becomes A + m_size_a = m_size_b; + m_start_a = m_start_b; + m_size_b = m_start_b = 0; + } } else { m_size_a -= size; m_start_a += size;