Merge #67707

67707: kv: fix aliasing bug in leaseHistory.get r=michae2 a=nvanbenschoten

This commit fixes a bug in `leaseHistory.get` where we were
unintentionally returning an aliased slice that was protected by the
struct's mutex instead of returning the new slice that we had already
copied into under lock. This bug has been around since the original
introduction of this leaseHistory type, over 4 years ago.

This was the likely cause of an `index out of range` crash we saw in a
production cluster that was serving a `RangesResponse` for a debug.zip.

Release note (bug fix): A rare bug that could result in a crash while
creating a debug.zip has been fixed. The bug was only possible to hit if
a debug.zip was captured during a period of rapid lease movement.

Co-authored-by: Nathan VanBenschoten <[email protected]>

pkg/kv/kvserver/lease_history.go

@@ -58,7 +58,7 @@ func (lh *leaseHistory) get() []roachpb.Lease {
 	if len(lh.history) < leaseHistoryMaxEntries || lh.index == 0 {
 		result := make([]roachpb.Lease, len(lh.history))
 		copy(result, lh.history)
-		return lh.history
+		return result
 	}
 	first := lh.history[lh.index:]
 	second := lh.history[:lh.index]

pkg/kv/kvserver/lease_history_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/stretchr/testify/require"
 )
 
 func TestLeaseHistory(t *testing.T) {
@@ -35,6 +36,9 @@ func TestLeaseHistory(t *testing.T) {
 			if e, a := int64(i-1), leases[len(leases)-1].Epoch; e != a {
 				t.Errorf("%d: expected newest lease to have epoch of %d , actual %d:\n%+v", i, e, a, leases)
 			}
+			require.NotSame(t, &history.history[0], &leases[0], "expected slice copy")
+		} else {
+			require.Nil(t, leases)
 		}
 
 		history.add(roachpb.Lease{
@@ -54,6 +58,7 @@ func TestLeaseHistory(t *testing.T) {
 		if e, a := int64(i+leaseHistoryMaxEntries-1), leases[leaseHistoryMaxEntries-1].Epoch; e != a {
 			t.Errorf("%d: expected newest lease to have epoch of %d , actual %d:\n%+v", i, e, a, leases)
 		}
+		require.NotSame(t, &history.history[0], &leases[0], "expected slice copy")
 
 		history.add(roachpb.Lease{
 			Epoch: int64(i + leaseHistoryMaxEntries),