Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ring buffer deadlock #511

Merged
merged 3 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions internals/servicelog/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ func (it *iterator) Close() error {
if it.rb == nil {
return nil
}
it.rb.iteratorMutex.Lock()
defer it.rb.iteratorMutex.Unlock()
it.rb.removeIterator(it)
close(it.nextChan)
it.rb = nil
Expand Down
22 changes: 11 additions & 11 deletions internals/servicelog/ringbuffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ func NewRingBuffer(size int) *RingBuffer {
func (rb *RingBuffer) Close() error {
rb.rwlock.Lock()
defer rb.rwlock.Unlock()
rb.iteratorMutex.Lock()
defer rb.iteratorMutex.Unlock()
if rb.writeClosed {
return nil
}
Expand All @@ -86,6 +88,8 @@ func (rb *RingBuffer) Write(p []byte) (written int, err error) {
}
defer func() {
if written > 0 {
rb.iteratorMutex.RLock()
defer rb.iteratorMutex.RUnlock()
rb.signalIterators()
}
}()
Expand Down Expand Up @@ -237,16 +241,18 @@ func (rb *RingBuffer) WriteTo(writer io.Writer, start RingPos) (next RingPos, n

// TailIterator returns an iterator from the tail of the buffer.
func (rb *RingBuffer) TailIterator() Iterator {
rb.rwlock.RLock()
defer rb.rwlock.RUnlock()
rb.iteratorMutex.Lock()
defer rb.iteratorMutex.Unlock()
start, _ := rb.Positions()
start := rb.readIndex
iter := &iterator{
rb: rb,
index: start,
nextChan: make(chan bool, 1),
closeChan: make(chan struct{}),
}
if rb.Closed() {
if rb.writeClosed {
close(iter.closeChan)
}
rb.iteratorList = append(rb.iteratorList, iter)
Expand All @@ -257,6 +263,8 @@ func (rb *RingBuffer) TailIterator() Iterator {
// If lines is greater than zero, the iterator will start that many lines
// backwards from the head.
func (rb *RingBuffer) HeadIterator(lines int) Iterator {
rb.rwlock.RLock()
defer rb.rwlock.RUnlock()
IronCore864 marked this conversation as resolved.
Show resolved Hide resolved
firstLine := rb.reverseLinePosition(lines)
rb.iteratorMutex.Lock()
defer rb.iteratorMutex.Unlock()
Expand All @@ -266,16 +274,14 @@ func (rb *RingBuffer) HeadIterator(lines int) Iterator {
nextChan: make(chan bool, 1),
closeChan: make(chan struct{}),
}
if rb.Closed() {
if rb.writeClosed {
close(iter.closeChan)
}
rb.iteratorList = append(rb.iteratorList, iter)
return iter
}

func (rb *RingBuffer) reverseLinePosition(n int) RingPos {
rb.rwlock.RLock()
defer rb.rwlock.RUnlock()
if n <= 0 {
return rb.writeIndex
}
Expand Down Expand Up @@ -322,8 +328,6 @@ func (rb *RingBuffer) discard(n int) error {
}

func (rb *RingBuffer) signalIterators() {
rb.iteratorMutex.RLock()
defer rb.iteratorMutex.RUnlock()
for _, iter := range rb.iteratorList {
select {
case iter.nextChan <- true:
Expand All @@ -339,8 +343,6 @@ func (rb *RingBuffer) signalIterators() {
}

func (rb *RingBuffer) releaseIterators() {
rb.iteratorMutex.Lock()
defer rb.iteratorMutex.Unlock()
for _, iter := range rb.iteratorList {
// Close closeChan if not already closed
select {
Expand All @@ -353,8 +355,6 @@ func (rb *RingBuffer) releaseIterators() {
}

func (rb *RingBuffer) removeIterator(iter *iterator) {
rb.iteratorMutex.Lock()
defer rb.iteratorMutex.Unlock()
for i, storedIter := range rb.iteratorList {
if iter != storedIter {
continue
Expand Down
Loading