From 001900e4e98a496a3d336e5cf456b9df4dd26807 Mon Sep 17 00:00:00 2001 From: mprahl Date: Tue, 16 Jul 2024 10:55:26 -0400 Subject: [PATCH] Allow calling Stop multiple times on RetryWatcher This makes the Stop method idempotent so that if Stop is called multiple times, it does not cause a panic due to closing a closed channel. Signed-off-by: mprahl Kubernetes-commit: a54ba917be42c941edf1a0359dced04e1a5e1d6f --- tools/watch/retrywatcher.go | 12 +++++++++++- tools/watch/retrywatcher_test.go | 2 ++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tools/watch/retrywatcher.go b/tools/watch/retrywatcher.go index d81dc435..8431d02f 100644 --- a/tools/watch/retrywatcher.go +++ b/tools/watch/retrywatcher.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "net/http" + "sync" "time" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -53,6 +54,7 @@ type RetryWatcher struct { stopChan chan struct{} doneChan chan struct{} minRestartDelay time.Duration + stopChanLock sync.Mutex } // NewRetryWatcher creates a new RetryWatcher. @@ -286,7 +288,15 @@ func (rw *RetryWatcher) ResultChan() <-chan watch.Event { // Stop implements Interface. func (rw *RetryWatcher) Stop() { - close(rw.stopChan) + rw.stopChanLock.Lock() + defer rw.stopChanLock.Unlock() + + // Prevent closing an already closed channel to prevent a panic + select { + case <-rw.stopChan: + default: + close(rw.stopChan) + } } // Done allows the caller to be notified when Retry watcher stops. diff --git a/tools/watch/retrywatcher_test.go b/tools/watch/retrywatcher_test.go index fff3a46c..297661aa 100644 --- a/tools/watch/retrywatcher_test.go +++ b/tools/watch/retrywatcher_test.go @@ -585,6 +585,8 @@ func TestRetryWatcherToFinishWithUnreadEvents(t *testing.T) { // Give the watcher a chance to get to sending events (blocking) time.Sleep(10 * time.Millisecond) + watcher.Stop() + // Verify a second stop does not cause a panic watcher.Stop() maxTime := time.Second