Skip to content

Commit c1a5373

Browse files
authored
Merge pull request #3764 from LronDC/automated-cherry-pick-of-#3123-upstream-release-1.5
Automated cherry pick of #3123: Avoid multiple executions after rlock in concurrent scenarios
2 parents 0d67175 + d68315e commit c1a5373

File tree

4 files changed

+24
-18
lines changed

4 files changed

+24
-18
lines changed

pkg/util/fedinformer/genericmanager/multi-cluster-manager.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,14 @@ func (m *multiClusterInformerManagerImpl) getManager(cluster string) (SingleClus
8585
}
8686

8787
func (m *multiClusterInformerManagerImpl) ForCluster(cluster string, client dynamic.Interface, defaultResync time.Duration) SingleClusterInformerManager {
88+
m.lock.Lock()
89+
defer m.lock.Unlock()
90+
8891
// If informer manager already exist, just return
89-
if manager, exist := m.getManager(cluster); exist {
92+
if manager, exist := m.managers[cluster]; exist {
9093
return manager
9194
}
9295

93-
m.lock.Lock()
94-
defer m.lock.Unlock()
9596
manager := NewSingleClusterInformerManager(client, defaultResync, m.stopCh)
9697
m.managers[cluster] = manager
9798
return manager

pkg/util/fedinformer/genericmanager/single-cluster-manager.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,11 @@ type singleClusterInformerManagerImpl struct {
8787
}
8888

8989
func (s *singleClusterInformerManagerImpl) ForResource(resource schema.GroupVersionResource, handler cache.ResourceEventHandler) {
90+
s.lock.Lock()
91+
defer s.lock.Unlock()
92+
9093
// if handler already exist, just return, nothing changed.
91-
if s.IsHandlerExist(resource, handler) {
94+
if s.isHandlerExist(resource, handler) {
9295
return
9396
}
9497

@@ -113,6 +116,10 @@ func (s *singleClusterInformerManagerImpl) IsHandlerExist(resource schema.GroupV
113116
s.lock.RLock()
114117
defer s.lock.RUnlock()
115118

119+
return s.isHandlerExist(resource, handler)
120+
}
121+
122+
func (s *singleClusterInformerManagerImpl) isHandlerExist(resource schema.GroupVersionResource, handler cache.ResourceEventHandler) bool {
116123
handlers, exist := s.handlers[resource]
117124
if !exist {
118125
return false
@@ -132,9 +139,6 @@ func (s *singleClusterInformerManagerImpl) Lister(resource schema.GroupVersionRe
132139
}
133140

134141
func (s *singleClusterInformerManagerImpl) appendHandler(resource schema.GroupVersionResource, handler cache.ResourceEventHandler) {
135-
s.lock.Lock()
136-
defer s.lock.Unlock()
137-
138142
// assume the handler list exist, caller should ensure for that.
139143
handlers := s.handlers[resource]
140144

pkg/util/fedinformer/typedmanager/multi-cluster-manager.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,14 @@ func (m *multiClusterInformerManagerImpl) getManager(cluster string) (SingleClus
9494
}
9595

9696
func (m *multiClusterInformerManagerImpl) ForCluster(cluster string, client kubernetes.Interface, defaultResync time.Duration) SingleClusterInformerManager {
97+
m.lock.Lock()
98+
defer m.lock.Unlock()
99+
97100
// If informer manager already exist, just return
98-
if manager, exist := m.getManager(cluster); exist {
101+
if manager, exist := m.managers[cluster]; exist {
99102
return manager
100103
}
101104

102-
m.lock.Lock()
103-
defer m.lock.Unlock()
104105
manager := NewSingleClusterInformerManager(client, defaultResync, m.stopCh, m.transformFuncs)
105106
m.managers[cluster] = manager
106107
return manager

pkg/util/fedinformer/typedmanager/single-cluster-manager.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,11 @@ type singleClusterInformerManagerImpl struct {
107107
}
108108

109109
func (s *singleClusterInformerManagerImpl) ForResource(resource schema.GroupVersionResource, handler cache.ResourceEventHandler) error {
110+
s.lock.Lock()
111+
defer s.lock.Unlock()
112+
110113
// if handler already exist, just return, nothing changed.
111-
if s.IsHandlerExist(resource, handler) {
114+
if s.isHandlerExist(resource, handler) {
112115
return nil
113116
}
114117

@@ -117,20 +120,16 @@ func (s *singleClusterInformerManagerImpl) ForResource(resource schema.GroupVers
117120
return err
118121
}
119122

120-
s.lock.Lock()
121123
if _, exist := s.informers[resource]; !exist {
122124
s.informers[resource] = struct{}{}
123125
}
124-
s.lock.Unlock()
125126

126-
s.lock.RLock()
127127
if resourceTransformFunc, ok := s.transformFuncs[resource]; ok && !s.isInformerStarted(resource) {
128128
err = resourceInformer.Informer().SetTransform(resourceTransformFunc)
129129
if err != nil {
130130
return err
131131
}
132132
}
133-
s.lock.RUnlock()
134133

135134
_, err = resourceInformer.Informer().AddEventHandler(handler)
136135
if err != nil {
@@ -159,6 +158,10 @@ func (s *singleClusterInformerManagerImpl) IsHandlerExist(resource schema.GroupV
159158
s.lock.RLock()
160159
defer s.lock.RUnlock()
161160

161+
return s.isHandlerExist(resource, handler)
162+
}
163+
164+
func (s *singleClusterInformerManagerImpl) isHandlerExist(resource schema.GroupVersionResource, handler cache.ResourceEventHandler) bool {
162165
handlers, exist := s.handlers[resource]
163166
if !exist {
164167
return false
@@ -205,9 +208,6 @@ func (s *singleClusterInformerManagerImpl) Lister(resource schema.GroupVersionRe
205208
}
206209

207210
func (s *singleClusterInformerManagerImpl) appendHandler(resource schema.GroupVersionResource, handler cache.ResourceEventHandler) {
208-
s.lock.Lock()
209-
defer s.lock.Unlock()
210-
211211
// assume the handler list exist, caller should ensure for that.
212212
handlers := s.handlers[resource]
213213

0 commit comments

Comments
 (0)