From c9066d75f6d23384df20be23557a26a4b9aac871 Mon Sep 17 00:00:00 2001
From: Ben Luddy <bluddy@redhat.com>
Date: Wed, 11 Dec 2024 18:38:09 -0500
Subject: [PATCH] Fix data race in CBOR serializer's custom marshaler type
 cache.

A placeholder entry is added to the cache while the permanent entry is being constructed. A data
race existed where the placeholder entry itself could be mutated after its address may have been
given to other callers.
---
 .../serializer/cbor/internal/modes/custom.go  |  4 +-
 .../cbor/internal/modes/custom_test.go        | 39 ++++++++++++++++---
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom.go
index 858529e958a..e550ea348dd 100644
--- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom.go
+++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom.go
@@ -140,7 +140,7 @@ func (cache *checkers) getCheckerInternal(rt reflect.Type, parent *path) (c chec
 	var wg sync.WaitGroup
 	wg.Add(1)
 	defer wg.Done()
-	c = checker{
+	placeholder := checker{
 		safe: func() bool {
 			wg.Wait()
 			return c.safe()
@@ -150,7 +150,7 @@ func (cache *checkers) getCheckerInternal(rt reflect.Type, parent *path) (c chec
 			return c.check(rv, depth)
 		},
 	}
-	if actual, loaded := cache.m.LoadOrStore(rt, &c); loaded {
+	if actual, loaded := cache.m.LoadOrStore(rt, &placeholder); loaded {
 		// Someone else stored an entry for this type, use it.
 		return *actual.(*checker)
 	}
diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom_test.go
index 80d55537e03..71f69b5e601 100644
--- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom_test.go
+++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom_test.go
@@ -14,15 +14,15 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package modes_test
+package modes
 
 import (
 	"encoding"
 	"encoding/json"
+	"reflect"
+	"sync"
 	"testing"
 
-	"k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes"
-
 	"github.com/fxamacker/cbor/v2"
 )
 
@@ -132,7 +132,7 @@ func TestCheckUnsupportedMarshalers(t *testing.T) {
 			SafeCyclicTypeA{},
 			SafeCyclicTypeB{},
 		} {
-			if err := modes.RejectCustomMarshalers(v); err != nil {
+			if err := RejectCustomMarshalers(v); err != nil {
 				t.Errorf("%#v: unexpected non-nil error: %v", v, err)
 			}
 		}
@@ -161,9 +161,38 @@ func TestCheckUnsupportedMarshalers(t *testing.T) {
 			UnsafeCyclicTypeA{Bs: []UnsafeCyclicTypeB{{}}},
 			UnsafeCyclicTypeB{},
 		} {
-			if err := modes.RejectCustomMarshalers(v); err == nil {
+			if err := RejectCustomMarshalers(v); err == nil {
 				t.Errorf("%#v: unexpected nil error", v)
 			}
 		}
 	})
 }
+
+// With -test.race, retrieves a custom marshaler checker for a cyclic type concurrently from many
+// goroutines to root out data races in checker lazy initialization.
+func TestLazyCheckerInitializationDataRace(t *testing.T) {
+	cache := checkers{
+		cborInterface: reflect.TypeFor[cbor.Marshaler](),
+		nonCBORInterfaces: []reflect.Type{
+			reflect.TypeFor[json.Marshaler](),
+			reflect.TypeFor[encoding.TextMarshaler](),
+		},
+	}
+
+	rt := reflect.TypeFor[SafeCyclicTypeA]()
+
+	begin := make(chan struct{})
+
+	var wg sync.WaitGroup
+	for range 32 {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			<-begin
+			cache.getChecker(rt)
+		}()
+	}
+
+	close(begin)
+	wg.Wait()
+}