Skip to content

Commit 885c7ba

Browse files
committed
fix concurrent map read access and write panic
The controller already uses the RWMutex to guard the calls to state.apply() (which is important because we have 2 worker threads in this controller), but failed to also guard the state.getGVKRs() call. apply() would write to the underlying map in the DefaultRESTMapper, getGVKRs() would read. Boom. This panic happens extremely rarely and only surfaced when I moved the initialisation of the DiscoveringDynamicSharedInformerFactory up from server.NewServer() into server.NewConfig(). With that change, kcp begun to panic in maybe 30% of cases within the first 5 seconds of runtime. Since this is in fact a race condition, this fix still make sense even when the panic barely ever surfaces right now. On-behalf-of: @SAP [email protected]
1 parent eadfe46 commit 885c7ba

File tree

1 file changed

+3
-0
lines changed

1 file changed

+3
-0
lines changed

pkg/reconciler/dynamicrestmapper/dynamicrestmapper_builtin_controller.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,9 @@ func (c *BuiltinTypesController) gatherGVKRsForCRD(crd *apiextensionsv1.CustomRe
201201
}
202202

203203
func (c *BuiltinTypesController) gatherGVKRsForMappedGroupResource(gr schema.GroupResource) ([]typeMeta, error) {
204+
c.state.lock.RLock()
205+
defer c.state.lock.RUnlock()
206+
204207
gvkrs, err := c.state.builtin.getGVKRs(gr)
205208
if err != nil {
206209
if meta.IsNoMatchError(err) {

0 commit comments

Comments
 (0)