diff --git a/db/skipped_sequence.go b/db/skipped_sequence.go index d2c54ac70f..687e9cf382 100644 --- a/db/skipped_sequence.go +++ b/db/skipped_sequence.go @@ -187,10 +187,11 @@ func (s *SkippedSequenceSlice) _clip(ctx context.Context) { threshold := 2.5 * float64(len(s.list)) if cap(s.list) > int(threshold) { + newCap := len(s.list) + s.ClipCapacityHeadroom // check if we can safely clip without an out of bound errors - if (len(s.list) + s.ClipCapacityHeadroom) < cap(s.list) { + if newCap < cap(s.list) { base.DebugfCtx(ctx, base.KeyCache, "clipping skipped list capacity") - s.list = s.list[:len(s.list):s.ClipCapacityHeadroom] + s.list = s.list[:len(s.list):newCap] } } } diff --git a/db/skipped_sequence_test.go b/db/skipped_sequence_test.go index cefc724042..58c671adee 100644 --- a/db/skipped_sequence_test.go +++ b/db/skipped_sequence_test.go @@ -604,11 +604,11 @@ func TestCompactSkippedListClipHandling(t *testing.T) { }{ { name: "single_items", - expectedCap: 100, + expectedCap: 101, }, { name: "range_items", - expectedCap: 100, + expectedCap: 101, }, } for _, testCase := range testCases { @@ -844,6 +844,52 @@ func TestRemoveSequenceRange(t *testing.T) { } +// TestClipSafety: +// - Test case 1: (CBG-3945) calling clip on slice that should be clipped, asserting that the new capacity doesn't cause +// out of bounds error. CBG-3945 was hit due to clip attempting to clip capacity to a value below the +// current length of the slice +// - Test case 2: calling clip on a slice that meets the threshold to be clipped but shouldn't as it would cause out of +// bounds error (length of list + headroom > the current capacity) +// - Test case 3: calling clip on slice that doesn't meet clip threshold, assert that it doesn't get clipped +func TestClipSafety(t *testing.T) { + testCases := []struct { + name string + inputList []*SkippedSequenceListEntry + expectedLen int + expectedCap int + }{ + { + name: "larger_length_than_headroom", + inputList: make([]*SkippedSequenceListEntry, 66291, 10000000), + expectedCap: 67291, + expectedLen: 66291, + }, + { + name: "above_threshold_capacity_new_cap_out_of_bounds", + inputList: make([]*SkippedSequenceListEntry, 1, 100), + expectedCap: 100, + expectedLen: 1, + }, + { + name: "threshold_not_met_for_clip", + inputList: make([]*SkippedSequenceListEntry, 100, 101), + expectedCap: 101, + expectedLen: 100, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + skippedSlice := NewSkippedSequenceSlice(DefaultClipCapacityHeadroom) + ctx := base.TestCtx(t) + skippedSlice.list = testCase.inputList + + skippedSlice._clip(ctx) + assert.Equal(t, testCase.expectedLen, len(skippedSlice.list)) + assert.Equal(t, testCase.expectedCap, cap(skippedSlice.list)) + }) + } +} + func TestProcessUnusedSequenceRangeAtSkipped(t *testing.T) { testCases := []struct { name string