Skip to content

Commit 5c207d6

Browse files
authored
Merge pull request kubernetes#129170 from benluddy/cyclic-marshaler-cache-race
Fix data race in CBOR serializer's custom marshaler type cache.
2 parents bcd65ce + c9066d7 commit 5c207d6

File tree

2 files changed

+36
-7
lines changed

2 files changed

+36
-7
lines changed

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func (cache *checkers) getCheckerInternal(rt reflect.Type, parent *path) (c chec
140140
var wg sync.WaitGroup
141141
wg.Add(1)
142142
defer wg.Done()
143-
c = checker{
143+
placeholder := checker{
144144
safe: func() bool {
145145
wg.Wait()
146146
return c.safe()
@@ -150,7 +150,7 @@ func (cache *checkers) getCheckerInternal(rt reflect.Type, parent *path) (c chec
150150
return c.check(rv, depth)
151151
},
152152
}
153-
if actual, loaded := cache.m.LoadOrStore(rt, &c); loaded {
153+
if actual, loaded := cache.m.LoadOrStore(rt, &placeholder); loaded {
154154
// Someone else stored an entry for this type, use it.
155155
return *actual.(*checker)
156156
}

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom_test.go

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package modes_test
17+
package modes
1818

1919
import (
2020
"encoding"
2121
"encoding/json"
22+
"reflect"
23+
"sync"
2224
"testing"
2325

24-
"k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes"
25-
2626
"github.com/fxamacker/cbor/v2"
2727
)
2828

@@ -132,7 +132,7 @@ func TestCheckUnsupportedMarshalers(t *testing.T) {
132132
SafeCyclicTypeA{},
133133
SafeCyclicTypeB{},
134134
} {
135-
if err := modes.RejectCustomMarshalers(v); err != nil {
135+
if err := RejectCustomMarshalers(v); err != nil {
136136
t.Errorf("%#v: unexpected non-nil error: %v", v, err)
137137
}
138138
}
@@ -161,9 +161,38 @@ func TestCheckUnsupportedMarshalers(t *testing.T) {
161161
UnsafeCyclicTypeA{Bs: []UnsafeCyclicTypeB{{}}},
162162
UnsafeCyclicTypeB{},
163163
} {
164-
if err := modes.RejectCustomMarshalers(v); err == nil {
164+
if err := RejectCustomMarshalers(v); err == nil {
165165
t.Errorf("%#v: unexpected nil error", v)
166166
}
167167
}
168168
})
169169
}
170+
171+
// With -test.race, retrieves a custom marshaler checker for a cyclic type concurrently from many
172+
// goroutines to root out data races in checker lazy initialization.
173+
func TestLazyCheckerInitializationDataRace(t *testing.T) {
174+
cache := checkers{
175+
cborInterface: reflect.TypeFor[cbor.Marshaler](),
176+
nonCBORInterfaces: []reflect.Type{
177+
reflect.TypeFor[json.Marshaler](),
178+
reflect.TypeFor[encoding.TextMarshaler](),
179+
},
180+
}
181+
182+
rt := reflect.TypeFor[SafeCyclicTypeA]()
183+
184+
begin := make(chan struct{})
185+
186+
var wg sync.WaitGroup
187+
for range 32 {
188+
wg.Add(1)
189+
go func() {
190+
defer wg.Done()
191+
<-begin
192+
cache.getChecker(rt)
193+
}()
194+
}
195+
196+
close(begin)
197+
wg.Wait()
198+
}

0 commit comments

Comments
 (0)