test: fix potential goroutine leak in TestUpdateAddresses_RetryFromFirstAddr

[charlesxsh]

This PR aims to fix possible goroutine leak in TestUpdateAddresses_RetryFromFirstAddr.

=== RUN TestUpdateAddresses_RetryFromFirstAddr
clientconn_test.go:xxxx: timed out waiting for server1 to be contacted
clientconn_test.go:xxxx: accept tcp 127.0.0.1:32795: use of closed network connection

goroutine 10 [chan receive]:
google.golang.org/grpc.TestUpdateAddresses_RetryFromFirstAddr.func1(0xab6d80, 0xc00000c888, 0xc0002c0a00, 0xc00002a880, 0xc00002a900)
	/fuzz/target/clientconn_test.go:1309 +0x205
created by google.golang.org/grpc.TestUpdateAddresses_RetryFromFirstAddr
	/fuzz/target/clientconn_test.go:1287 +0x60a

Any fatal happened later, like following example:

select {
	case <-server1ContactedFirstTime:
	case <-timeout:
		t.Fatal("timed out waiting for server1 to be contacted")
	}

If fatal happened, server1 goroutine will still be blocked at range stateNotifications since no close operation will be made.

for s := range stateNotifications {

RELEASE NOTES: none

[menghanl]

Can you share how you made this test fail?
Is it flaky (and you just ran it many times)? Or you had to change some code to make it fail?

If I understand correctly, this leak only happens when the test fails. I would think it's OK to have a leak in a failing test.

[github-actions]

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

[charlesxsh]

@menghanl This issue is found by our tool through fuzzing. In this case, it is triggered by Fatal call in the test, however, it can also be triggered by any unexpected exit. So it might be better fixed instead of letting it be there?

[menghanl]

How does this fuzzing work? Does it change just the data or the code?
If there's a situation this test can fatal, that's a more serious problem and we should fix that first.

[charlesxsh]

Oh no worry, nothing wrong with the test or business logic. It instrumented the code. It simply let us see the result in some corner case.

[songlh]

There is another problem in this unit test. If close(closeServer2) (line 990) is skipped due to panics, the goroutine created at line 915 is leaked.

[menghanl]

I think this PR is good as is. Let's make separate PRs if there are further improvements.