fix data race in Source.linesInRange under in-place compaction

In-place compaction in Append overwrites and then clears positions in the
backing array. linesInRange returned a slice aliased into that array and
released the read lock before the caller iterated it, so a concurrent
compaction could nil out the caller's view (panic on DisplayString()) and
trip the race detector. Return a copy.

Add a -race regression test that pins the contract.
This commit is contained in:
Daisuke Maki 2026-06-04 10:14:55 +09:00
parent ac77b9f2ff
commit 886f02bdee
2 changed files with 55 additions and 3 deletions

View file

@ -324,12 +324,17 @@ func (s *Source) SetupDone() <-chan struct{} {
return s.setupDone
}
// linesInRange returns a slice of lines between from and to (indices into the
// live window) from the buffer.
// linesInRange returns the lines between from and to (indices into the live
// window) from the buffer. The returned slice is a copy: Append may compact
// the backing array in place after the lock is released, which would
// otherwise overwrite or clear the slice the caller is still iterating.
func (s *Source) linesInRange(from, to int) []line.Line {
s.mutex.RLock()
defer s.mutex.RUnlock()
return s.lines[s.start+from : s.start+to]
src := s.lines[s.start+from : s.start+to]
out := make([]line.Line, len(src))
copy(out, src)
return out
}
// LineAt returns the line at the given index (within the live window) from the buffer.

View file

@ -3,6 +3,7 @@ package peco
import (
"fmt"
"strings"
"sync"
"testing"
"github.com/peco/peco/line"
@ -74,3 +75,49 @@ func TestSourceUnlimitedRetainsAll(t *testing.T) {
require.NoError(t, err)
require.Equal(t, fmt.Sprintf("line%d", total-1), last.DisplayString())
}
// TestSourceLinesInRangeConcurrentAppend verifies that the slice returned by
// linesInRange remains stable for the caller even when Append concurrently
// compacts the backing array. Run under -race, this catches any future
// regression where linesInRange returns a slice aliased into the live storage.
func TestSourceLinesInRangeConcurrentAppend(t *testing.T) {
const capacity = 32
ig := newIDGen()
go ig.Run(t.Context())
s := NewSource("-", strings.NewReader(""), false, ig, capacity, false, false)
for i := range capacity {
s.Append(line.NewRaw(uint64(i), fmt.Sprintf("seed%d", i), false, false))
}
stop := make(chan struct{})
var wg sync.WaitGroup
wg.Go(func() {
i := uint64(capacity)
for {
select {
case <-stop:
return
default:
}
s.Append(line.NewRaw(i, fmt.Sprintf("hot%d", i), false, false))
i++
}
})
// Repeatedly grab a window and read every element. If linesInRange
// returned a slice aliased into s.lines, a concurrent compaction would
// either nil out entries (post-clear) or rewrite them, producing a race
// detector hit and/or a nil DisplayString() deref.
for range 2000 {
rng := s.linesInRange(0, capacity)
require.Len(t, rng, capacity)
for _, l := range rng {
require.NotNil(t, l)
_ = l.DisplayString()
}
}
close(stop)
wg.Wait()
}