From 5f6ea627a306fb43193e01935d0bfd21dd287b76 Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Sat, 27 May 2017 23:34:58 +1000 Subject: [PATCH] Shared Informer Run blocks until all goroutines finish Fixes #45454 Kubernetes-commit: d789615902ead2a112ad24fbfebe95285b87004b --- tools/cache/controller.go | 5 ++++- tools/cache/mutation_detector.go | 16 +++++++--------- tools/cache/reflector.go | 17 +++-------------- tools/cache/reflector_test.go | 2 +- tools/cache/shared_informer.go | 30 ++++++++++++++++++------------ 5 files changed, 33 insertions(+), 37 deletions(-) diff --git a/tools/cache/controller.go b/tools/cache/controller.go index 2c97b865..88003ce8 100644 --- a/tools/cache/controller.go +++ b/tools/cache/controller.go @@ -116,7 +116,10 @@ func (c *controller) Run(stopCh <-chan struct{}) { c.reflector = r c.reflectorMutex.Unlock() - r.RunUntil(stopCh) + var wg sync.WaitGroup + defer wg.Wait() + + wait.StartUntil(stopCh, &wg, r.Run) wait.Until(c.processLoop, time.Second, stopCh) } diff --git a/tools/cache/mutation_detector.go b/tools/cache/mutation_detector.go index cc6094ce..8c067cab 100644 --- a/tools/cache/mutation_detector.go +++ b/tools/cache/mutation_detector.go @@ -79,17 +79,15 @@ type cacheObj struct { func (d *defaultCacheMutationDetector) Run(stopCh <-chan struct{}) { // we DON'T want protection from panics. If we're running this code, we want to die - go func() { - for { - d.CompareObjects() + for { + d.CompareObjects() - select { - case <-stopCh: - return - case <-time.After(d.period): - } + select { + case <-stopCh: + return + case <-time.After(d.period): } - }() + } } // AddObject makes a deep copy of the object for later comparison. It only works on runtime.Object diff --git a/tools/cache/reflector.go b/tools/cache/reflector.go index 9a730610..c09a7865 100644 --- a/tools/cache/reflector.go +++ b/tools/cache/reflector.go @@ -182,21 +182,10 @@ func extractStackCreator() (string, int, bool) { } // Run starts a watch and handles watch events. Will restart the watch if it is closed. -// Run starts a goroutine and returns immediately. -func (r *Reflector) Run() { +// Run will exit when stopCh is closed. +func (r *Reflector) Run(stopCh <-chan struct{}) { glog.V(3).Infof("Starting reflector %v (%s) from %s", r.expectedType, r.resyncPeriod, r.name) - go wait.Until(func() { - if err := r.ListAndWatch(wait.NeverStop); err != nil { - utilruntime.HandleError(err) - } - }, r.period, wait.NeverStop) -} - -// RunUntil starts a watch and handles watch events. Will restart the watch if it is closed. -// RunUntil starts a goroutine and returns immediately. It will exit when stopCh is closed. -func (r *Reflector) RunUntil(stopCh <-chan struct{}) { - glog.V(3).Infof("Starting reflector %v (%s) from %s", r.expectedType, r.resyncPeriod, r.name) - go wait.Until(func() { + wait.Until(func() { if err := r.ListAndWatch(stopCh); err != nil { utilruntime.HandleError(err) } diff --git a/tools/cache/reflector_test.go b/tools/cache/reflector_test.go index c092beca..bb06059f 100644 --- a/tools/cache/reflector_test.go +++ b/tools/cache/reflector_test.go @@ -83,7 +83,7 @@ func TestRunUntil(t *testing.T) { return &v1.PodList{ListMeta: metav1.ListMeta{ResourceVersion: "1"}}, nil }, } - r.RunUntil(stopCh) + go r.Run(stopCh) // Synchronously add a dummy pod into the watch channel so we // know the RunUntil go routine is in the watch handler. fw.Add(&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}}) diff --git a/tools/cache/shared_informer.go b/tools/cache/shared_informer.go index a0dbbb69..935f9d36 100644 --- a/tools/cache/shared_informer.go +++ b/tools/cache/shared_informer.go @@ -147,6 +147,7 @@ type sharedIndexInformer struct { // stopCh is the channel used to stop the main Run process. We have to track it so that // late joiners can have a proper stop stopCh <-chan struct{} + wg sync.WaitGroup } // dummyController hides the fact that a SharedInformer is different from a dedicated one @@ -204,12 +205,14 @@ func (s *sharedIndexInformer) Run(stopCh <-chan struct{}) { s.controller = New(cfg) s.controller.(*controller).clock = s.clock + s.stopCh = stopCh s.started = true }() - s.stopCh = stopCh - s.cacheMutationDetector.Run(stopCh) - s.processor.run(stopCh) + defer s.wg.Wait() + + wait.StartUntil(stopCh, &s.wg, s.cacheMutationDetector.Run) + wait.StartUntil(stopCh, &s.wg, s.processor.run) s.controller.Run(stopCh) } @@ -324,8 +327,8 @@ func (s *sharedIndexInformer) AddEventHandlerWithResyncPeriod(handler ResourceEv s.processor.addListener(listener) - go listener.run(s.stopCh) - go listener.pop(s.stopCh) + wait.StartUntil(s.stopCh, &s.wg, listener.run) + wait.StartUntil(s.stopCh, &s.wg, listener.pop) items := s.indexer.List() for i := range items { @@ -395,13 +398,16 @@ func (p *sharedProcessor) distribute(obj interface{}, sync bool) { } func (p *sharedProcessor) run(stopCh <-chan struct{}) { - p.listenersLock.RLock() - defer p.listenersLock.RUnlock() - - for _, listener := range p.listeners { - go listener.run(stopCh) - go listener.pop(stopCh) - } + var wg sync.WaitGroup + func() { + p.listenersLock.RLock() + defer p.listenersLock.RUnlock() + for _, listener := range p.listeners { + wait.StartUntil(stopCh, &wg, listener.run) + wait.StartUntil(stopCh, &wg, listener.pop) + } + }() + wg.Wait() } // shouldResync queries every listener to determine if any of them need a resync, based on each