From 2231ff5ae4175146c3f3d200185234c33c858bc2 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 1 Dec 2023 18:35:28 +0100 Subject: [PATCH] client-go events: support context.Background() as context If, for whatever reason, the context was context.Background(), the additional goroutine was started and then got stuck forever because context.Background().Done() is a nil channel. Found when indirectly instantiating a broadcaster with such a context: found unexpected goroutines: [Goroutine 9106 in state chan receive (nil chan), with k8s.io/kubernetes/vendor/k8s.io/client-go/tools/record.NewBroadcaster.func1 on top of the stack: goroutine 9106 [chan receive (nil chan)]: k8s.io/kubernetes/vendor/k8s.io/client-go/tools/record.NewBroadcaster.func1() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/client-go/tools/record/event.go:206 +0x2c created by k8s.io/kubernetes/vendor/k8s.io/client-go/tools/record.NewBroadcaster in goroutine 8957 /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/client-go/tools/record/event.go:205 +0x1a5 This can be fixed by checking for a nil channel. Another problem also gets addressed: if Shutdown was called without canceling the context, the goroutine also didn't stop. Now it waits for the cancelation context and thus terminates in both cases. Kubernetes-commit: eed6e29a5b8cfaa20fbc426541d9c74105d430ee --- tools/record/event.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tools/record/event.go b/tools/record/event.go index d1511696..0745fb4a 100644 --- a/tools/record/event.go +++ b/tools/record/event.go @@ -198,16 +198,29 @@ func NewBroadcaster(opts ...BroadcasterOption) EventBroadcaster { ctx := c.Context if ctx == nil { ctx = context.Background() - } else { + } + // The are two scenarios where it makes no sense to wait for context cancelation: + // - The context was nil. + // - The context was context.Background() to begin with. + // + // Both cases get checked here. + haveCtxCancelation := ctx.Done() == nil + + eventBroadcaster.cancelationCtx, eventBroadcaster.cancel = context.WithCancel(ctx) + + if haveCtxCancelation { // Calling Shutdown is not required when a context was provided: // when the context is canceled, this goroutine will shut down // the broadcaster. + // + // If Shutdown is called first, then this goroutine will + // also stop. go func() { - <-ctx.Done() + <-eventBroadcaster.cancelationCtx.Done() eventBroadcaster.Broadcaster.Shutdown() }() } - eventBroadcaster.cancelationCtx, eventBroadcaster.cancel = context.WithCancel(ctx) + return eventBroadcaster }