Repository: kubernetes/kubernetes (language: go)

Issue:
k8s.io/kubernetes/pkg/controller UT flakes for invalid memory address
### Which jobs are flaking?

- k8s.io/kubernetes/pkg/controller/garbagecollector.garbagecollector
- https://k8s.io/kubernetes/pkg/controller: tainteviction

### Which tests are flaking?

https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-kubernetes-unit/1795460000760467456
https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-kubernetes-unit/1795613262369263616

### Since when has it been flaking?

NA

### Testgrid link

https://testgrid.k8s.io/sig-release-master-blocking#ci-kubernetes-unit

### Reason for failure (if possible)

> stacktrace=<
> 	goroutine 561 [running]:
> 	k8s.io/apimachinery/pkg/util/runtime.logPanic({0x3a488b8, 0x4edba40}, {0x31ac240, 0x4e2c1d0})
> 		/home/prow/go/src/k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go:107 +0xe5
> 	k8s.io/apimachinery/pkg/util/runtime.handleCrash({0x3a488b8, 0x4edba40}, {0x31ac240, 0x4e2c1d0}, {0x4edba40, 0x0, 0xc000c94da8?})
> 		/home/prow/go/src/k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go:82 +0xa2
> 	k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0x36477f0?})
> 		/home/prow/go/src/k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go:59 +0x18f
> 	panic({0x31ac240?, 0x4e2c1d0?})
> 		/usr/local/go/src/runtime/panic.go:770 +0x132
> 	k8s.io/client-go/tools/record.(*eventBroadcasterImpl).StartEventWatcher.func1()
> 		/home/prow/go/src/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/record/event.go:407 +0xbc
> 	created by k8s.io/client-go/tools/record.(*eventBroadcasterImpl).StartEventWatcher in goroutine 552
> 		/home/prow/go/src/k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/record/event.go:400 +0x1b1
>  >
> panic: runtime error: invalid memory address or nil pointer dereference [recovered]
> 	panic: runtime error: invalid memory address or nil pointer dereference
> [signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x223479c]
> 

### Anything else we need to know?

_No response_

### Relevant SIG(s)

_No response_


Discussion / hints:
This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the `triage/accepted` label and provide further guidance.

The `triage/accepted` label can be added by org members by writing `/triage accepted` in a comment.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
The panic is in the event watcher, @pohly @dgrisonnet you have landed several changes recently that are most probably related, can  you PTAL?

/sig instrumentation
ok, so the problem is this

> E0528 14:33:05.634743   48023 event.go:398] "Unable start event watcher (will not retry!)" err="broadcaster already stopped" logger="TestGarbageCollectorConstruction leaked goroutine"
> E0528 14:33:05.635289   48023 event.go:398] "Unable start event watcher (will not retry!)" err="broadcaster already stopped" logger="TestGarbageCollectorConstruction leaked goroutine"


The watcher fails to be created

https://github.com/kubernetes/kubernetes/blob/7ba244f50e57257e9d435b42f3b0f17a92a0611a/staging/src/k8s.io/client-go/tools/record/event.go#L397-L399

but the goroutine keeps going and tries to get a channel from it

https://github.com/kubernetes/kubernetes/blob/7ba244f50e57257e9d435b42f3b0f17a92a0611a/staging/src/k8s.io/client-go/tools/record/event.go#L407

/assign @pohly 
That problem has already been there before the recent changes:

https://github.com/kubernetes/kubernetes/blob/f7dacb63cc0b0e79450b2f6d97b1c6e6c46070bd/staging/src/k8s.io/client-go/tools/record/event.go#L313-L331

When constructing the watch fails, that code needs to return instead of spawning the goroutine.

I think this is okay:
> E0528 14:33:05.634743 48023 event.go:398] "Unable start event watcher (will not retry!)" err="broadcaster already stopped" logger="TestGarbageCollectorConstruction leaked goroutine"

The `logger="TestGarbageCollectorConstruction leaked goroutine"` indicates that the `Unable start event watcher (will not retry!)` occurs after the test has completed. Before the recent changes, creating the watch would have succeeded and the test would have permanently leaked the goroutines. In that sense, the recent changes work as intended.


ok, so this just uncovered an existing bug, those are the positive news 😄 
> When constructing the watch fails, that code needs to return instead of spawning the goroutine.

I don't know if we'll translate the problem to other layer, as `StartEventWatcher` will return a nil ,  it seems StartEventWatcher has to also return an error?
I'm adding `return watch.NewEmptyWatch()` - https://github.com/kubernetes/kubernetes/blob/afebfdc5d411859468153e2b9921915f3e4f9ff4/staging/src/k8s.io/apimachinery/pkg/watch/watch.go#L74-L81

Fix patch:
diff --git a/staging/src/k8s.io/client-go/tools/record/event.go b/staging/src/k8s.io/client-go/tools/record/event.go
index 0b3b14f8da001..55947d2094b65 100644
--- a/staging/src/k8s.io/client-go/tools/record/event.go
+++ b/staging/src/k8s.io/client-go/tools/record/event.go
@@ -395,7 +395,11 @@ func (e *eventBroadcasterImpl) StartStructuredLogging(verbosity klog.Level) watc
 func (e *eventBroadcasterImpl) StartEventWatcher(eventHandler func(*v1.Event)) watch.Interface {
 	watcher, err := e.Watch()
 	if err != nil {
+		// This function traditionally returns no error even though it can fail.
+		// Instead, it logs the error and returns an empty watch. The empty
+		// watch ensures that callers don't crash when calling Stop.
 		klog.FromContext(e.cancelationCtx).Error(err, "Unable start event watcher (will not retry!)")
+		return watch.NewEmptyWatch()
 	}
 	go func() {
 		defer utilruntime.HandleCrash()


Test patch: