Skip to content

Commit d8aa5fb

Browse files
committed
fix(store): remove canceled height waiters
Signed-off-by: luangucun <luangucun@outlook.com>
1 parent cca63ca commit d8aa5fb

2 files changed

Lines changed: 119 additions & 0 deletions

File tree

pkg/store/store_adapter.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,37 @@ func (hs *heightSub) Wait(ctx context.Context, height uint64) error {
104104
case <-ch:
105105
return nil
106106
case <-ctx.Done():
107+
hs.removeWaiter(height, ch)
107108
return ctx.Err()
108109
}
109110
}
110111

112+
// removeWaiter unregisters a specific waiter from the requested height.
113+
func (hs *heightSub) removeWaiter(height uint64, target chan struct{}) {
114+
hs.heightMu.Lock()
115+
defer hs.heightMu.Unlock()
116+
117+
chs, ok := hs.heightChs[height]
118+
if !ok {
119+
return
120+
}
121+
122+
for i, ch := range chs {
123+
if ch != target {
124+
continue
125+
}
126+
copy(chs[i:], chs[i+1:])
127+
chs[len(chs)-1] = nil
128+
chs = chs[:len(chs)-1]
129+
if len(chs) == 0 {
130+
delete(hs.heightChs, height)
131+
} else {
132+
hs.heightChs[height] = chs
133+
}
134+
return
135+
}
136+
}
137+
111138
// notifyUpTo notifies all waiters for heights <= h.
112139
func (hs *heightSub) notifyUpTo(h uint64) {
113140
hs.heightMu.Lock()

pkg/store/store_adapter_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"sync"
66
"testing"
7+
"time"
78

89
"github.com/stretchr/testify/assert"
910
"github.com/stretchr/testify/require"
@@ -311,6 +312,97 @@ func TestPendingCache_ConcurrentAccess(t *testing.T) {
311312
assert.GreaterOrEqual(t, cache.len(), 0)
312313
}
313314

315+
func TestHeightSubWaitCancellationRemovesWaiter(t *testing.T) {
316+
t.Parallel()
317+
318+
hs := newHeightSub(1)
319+
for range 100 {
320+
ctx, cancel := context.WithCancel(context.Background())
321+
cancel()
322+
323+
require.ErrorIs(t, hs.Wait(ctx, 1_000), context.Canceled)
324+
}
325+
326+
hs.heightMu.Lock()
327+
defer hs.heightMu.Unlock()
328+
assert.Empty(t, hs.heightChs)
329+
}
330+
331+
func TestHeightSubWaitCancellationPreservesOtherWaiters(t *testing.T) {
332+
t.Parallel()
333+
334+
hs := newHeightSub(1)
335+
ctx1, cancel1 := context.WithCancel(context.Background())
336+
ctx2 := context.Background()
337+
waitDone := make(chan error, 2)
338+
go func() {
339+
waitDone <- hs.Wait(ctx1, 1_000)
340+
}()
341+
go func() {
342+
waitDone <- hs.Wait(ctx2, 1_000)
343+
}()
344+
345+
require.Eventually(t, func() bool {
346+
hs.heightMu.Lock()
347+
defer hs.heightMu.Unlock()
348+
return len(hs.heightChs[1_000]) == 2
349+
}, time.Second, time.Millisecond)
350+
351+
cancel1()
352+
require.Eventually(t, func() bool {
353+
hs.heightMu.Lock()
354+
defer hs.heightMu.Unlock()
355+
return len(hs.heightChs[1_000]) == 1
356+
}, time.Second, time.Millisecond)
357+
358+
hs.SetHeight(1_000)
359+
results := []error{<-waitDone, <-waitDone}
360+
assert.Contains(t, results, context.Canceled)
361+
assert.Contains(t, results, nil)
362+
}
363+
364+
func TestHeightSubWaitCancellationAndSetHeightConcurrent(t *testing.T) {
365+
t.Parallel()
366+
367+
for range 100 {
368+
hs := newHeightSub(1)
369+
ctx, cancel := context.WithCancel(context.Background())
370+
waitDone := make(chan error, 1)
371+
go func() {
372+
waitDone <- hs.Wait(ctx, 1_000)
373+
}()
374+
375+
// Ensure the waiter is registered before racing cancellation and notification.
376+
require.Eventually(t, func() bool {
377+
hs.heightMu.Lock()
378+
defer hs.heightMu.Unlock()
379+
return len(hs.heightChs[1_000]) == 1
380+
}, time.Second, time.Millisecond)
381+
382+
var wg sync.WaitGroup
383+
wg.Add(2)
384+
go func() {
385+
defer wg.Done()
386+
cancel()
387+
}()
388+
go func() {
389+
defer wg.Done()
390+
hs.SetHeight(1_000)
391+
}()
392+
wg.Wait()
393+
394+
err := <-waitDone
395+
if err != nil {
396+
assert.ErrorIs(t, err, context.Canceled)
397+
}
398+
399+
cancel()
400+
hs.heightMu.Lock()
401+
assert.Empty(t, hs.heightChs)
402+
hs.heightMu.Unlock()
403+
}
404+
}
405+
314406
// TestStoreAdapter_Backpressure tests that Append blocks when cache is full
315407
func TestStoreAdapter_Backpressure(t *testing.T) {
316408
t.Parallel()

0 commit comments

Comments
 (0)