Merge pull request #129170 from benluddy/cyclic-marshaler-cache-race

Fix data race in CBOR serializer's custom marshaler type cache.
This commit is contained in:
Kubernetes Prow Robot 2024-12-12 15:02:26 +01:00 committed by GitHub
commit 5c207d6fb0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 36 additions and 7 deletions

View File

@ -140,7 +140,7 @@ func (cache *checkers) getCheckerInternal(rt reflect.Type, parent *path) (c chec
var wg sync.WaitGroup var wg sync.WaitGroup
wg.Add(1) wg.Add(1)
defer wg.Done() defer wg.Done()
c = checker{ placeholder := checker{
safe: func() bool { safe: func() bool {
wg.Wait() wg.Wait()
return c.safe() return c.safe()
@ -150,7 +150,7 @@ func (cache *checkers) getCheckerInternal(rt reflect.Type, parent *path) (c chec
return c.check(rv, depth) 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. // Someone else stored an entry for this type, use it.
return *actual.(*checker) return *actual.(*checker)
} }

View File

@ -14,15 +14,15 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
package modes_test package modes
import ( import (
"encoding" "encoding"
"encoding/json" "encoding/json"
"reflect"
"sync"
"testing" "testing"
"k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes"
"github.com/fxamacker/cbor/v2" "github.com/fxamacker/cbor/v2"
) )
@ -132,7 +132,7 @@ func TestCheckUnsupportedMarshalers(t *testing.T) {
SafeCyclicTypeA{}, SafeCyclicTypeA{},
SafeCyclicTypeB{}, SafeCyclicTypeB{},
} { } {
if err := modes.RejectCustomMarshalers(v); err != nil { if err := RejectCustomMarshalers(v); err != nil {
t.Errorf("%#v: unexpected non-nil error: %v", v, err) t.Errorf("%#v: unexpected non-nil error: %v", v, err)
} }
} }
@ -161,9 +161,38 @@ func TestCheckUnsupportedMarshalers(t *testing.T) {
UnsafeCyclicTypeA{Bs: []UnsafeCyclicTypeB{{}}}, UnsafeCyclicTypeA{Bs: []UnsafeCyclicTypeB{{}}},
UnsafeCyclicTypeB{}, UnsafeCyclicTypeB{},
} { } {
if err := modes.RejectCustomMarshalers(v); err == nil { if err := RejectCustomMarshalers(v); err == nil {
t.Errorf("%#v: unexpected nil error", v) 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()
}