From 52b7d035f9655d0c6ebeaaafd60add99700bb468 Mon Sep 17 00:00:00 2001 From: David Bimmler Date: Thu, 28 Aug 2025 03:30:16 +0200 Subject: [PATCH] cache: do not allocate chan for nothing (#133500) * cache: do not allocate chan for nothing The explicit purpose of this channel is to never be closed and nothing to be sent down on it. Hence, there's no need to allocate a channel - a nil channel has exactly the desired behaviour. Additionally, this is more relevant now that testing/synctest gets unhappy when goroutines are blocked on reading on channels which are created outside of the synctest bubble. Since this is a package var, that's hard to avoid when using this package. Synctest is fine with nil channels though. Reported-by: Jussi Maki Signed-off-by: David Bimmler * handlers: do not allocate chan for nothing Nil chan has the desired semantics already, and this breaks testing/synctest because the channel is allocated outside of the bubble. Signed-off-by: David Bimmler --------- Signed-off-by: David Bimmler --- .../apiserver/pkg/endpoints/handlers/watch.go | 6 ++---- .../src/k8s.io/client-go/tools/cache/reflector.go | 14 +++++--------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go index 7bf702c6188..3f150a4f27f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go @@ -39,9 +39,6 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" ) -// nothing will ever be sent down this channel -var neverExitWatch <-chan time.Time = make(chan time.Time) - // timeoutFactory abstracts watch timeout logic for testing type TimeoutFactory interface { TimeoutCh() (<-chan time.Time, func() bool) @@ -56,7 +53,8 @@ type realTimeoutFactory struct { // and a cleanup function to call when this happens. func (w *realTimeoutFactory) TimeoutCh() (<-chan time.Time, func() bool) { if w.timeout == 0 { - return neverExitWatch, func() bool { return false } + // nothing will ever be sent down this channel + return nil, func() bool { return false } } t := time.NewTimer(w.timeout) return t.C, t.Stop diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector.go b/staging/src/k8s.io/client-go/tools/cache/reflector.go index ee9be77278a..9b33e01ef2d 100644 --- a/staging/src/k8s.io/client-go/tools/cache/reflector.go +++ b/staging/src/k8s.io/client-go/tools/cache/reflector.go @@ -49,11 +49,9 @@ import ( const defaultExpectedTypeName = "" -var ( - // We try to spread the load on apiserver by setting timeouts for - // watch requests - it is random in [minWatchTimeout, 2*minWatchTimeout]. - defaultMinWatchTimeout = 5 * time.Minute -) +// We try to spread the load on apiserver by setting timeouts for +// watch requests - it is random in [minWatchTimeout, 2*minWatchTimeout]. +var defaultMinWatchTimeout = 5 * time.Minute // ReflectorStore is the subset of cache.Store that the reflector uses type ReflectorStore interface { @@ -365,9 +363,6 @@ func (r *Reflector) RunWithContext(ctx context.Context) { } var ( - // nothing will ever be sent down this channel - neverExitWatch <-chan time.Time = make(chan time.Time) - // Used to indicate that watching stopped because of a signal from the stop // channel passed in from a client of the reflector. errorStopRequested = errors.New("stop requested") @@ -377,7 +372,8 @@ var ( // required, and a cleanup function. func (r *Reflector) resyncChan() (<-chan time.Time, func() bool) { if r.resyncPeriod == 0 { - return neverExitWatch, func() bool { return false } + // nothing will ever be sent down this channel + return nil, func() bool { return false } } // The cleanup function is required: imagine the scenario where watches // always fail so we end up listing frequently. Then, if we don't