Skip to content

Conversation

@twz123
Copy link
Contributor

@twz123 twz123 commented Dec 19, 2025

On some platforms, dialing 127.0.0.1:0 doesn't fail immediately but rather blocks until it times out. This leads to test failures because the leak detector identifies stray goroutines.

Even if the tests don't need the dial to succeed, create a random listener and use that as resolved address.

RELEASE NOTES: none

@codecov
Copy link

codecov bot commented Dec 19, 2025

Codecov Report

❌ Patch coverage is 89.23077% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.28%. Comparing base (25dbb81) to head (e8e0327).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
clientconn.go 80.00% 5 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8782      +/-   ##
==========================================
- Coverage   83.29%   83.28%   -0.01%     
==========================================
  Files         414      414              
  Lines       32753    32793      +40     
==========================================
+ Hits        27280    27312      +32     
- Misses       4070     4080      +10     
+ Partials     1403     1401       -2     
Files with missing lines Coverage Δ
balancer_wrapper.go 85.34% <100.00%> (+1.93%) ⬆️
internal/testutils/pipe_listener.go 84.61% <100.00%> (+0.83%) ⬆️
test/rawConnWrapper.go 66.10% <100.00%> (+0.28%) ⬆️
clientconn.go 90.16% <80.00%> (+0.18%) ⬆️

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@twz123 twz123 force-pushed the channelz-test-direct-error branch from 5e7db39 to b46a30e Compare December 19, 2025 16:32
@mbissa mbissa assigned mbissa and eshitachandwani and unassigned mbissa Dec 24, 2025
@eshitachandwani eshitachandwani added this to the 1.79 Release milestone Dec 31, 2025
@eshitachandwani
Copy link
Member

Instead of failing unconditionally , would a better way be to create a net.Listener and and pass it's address like :

lis, err := net.Listen("tcp", "localhost:0")
if err != nil {
    t.Fatalf("Failed to listen: %v", err)
}
defer lis.Close()

And then pass the address as

	resolvedAddrs := []resolver.Address{{Addr: lis.Addr().String(), ServerName: "grpclb.server"}}

The health check goroutine could outlive a call to ClientConn.close().
Add a done channel that will be waited on when closing the transport.

RELEASE NOTES:
- Closing a client connection will now block until the health check
  goroutine completes.

Signed-off-by: Tom Wieczorek <[email protected]>
@twz123 twz123 force-pushed the channelz-test-direct-error branch 2 times, most recently from ce50ee3 to b6734d9 Compare January 5, 2026 10:29
@twz123
Copy link
Contributor Author

twz123 commented Jan 5, 2026

Instead of failing unconditionally , would a better way be to create a net.Listener and and pass it's address like :

lis, err := net.Listen("tcp", "localhost:0")
if err != nil {
    t.Fatalf("Failed to listen: %v", err)
}
defer lis.Close()

And then pass the address as

	resolvedAddrs := []resolver.Address{{Addr: lis.Addr().String(), ServerName: "grpclb.server"}}

Right. I've tried this, and, funnily enough, when I allow the tests to dial, I encounter leak detector errors due to #8655. However, if I merge #8666 into this PR, the leak detector errors disappear.

I'll mark this one as draft until #8655 has been sorted out.

@twz123 twz123 marked this pull request as draft January 5, 2026 10:30
This will properly pass on cancellation requests, and reduce usage of a
deprecated method.

Signed-off-by: Tom Wieczorek <[email protected]>
@github-actions
Copy link

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.

@github-actions github-actions bot added stale and removed stale labels Jan 11, 2026
…tdialer' into clientconn-close-waitfor-goroutines
On some platforms, dialing 127.0.0.1:0 doesn't fail immediately but
rather blocks until it times out. This leads to test failures because
the leak detector identifies stray goroutines.

Even if the tests don't need the dial to succeed, create a random
listener and use that as resolved address.

Signed-off-by: Tom Wieczorek <[email protected]>
@twz123 twz123 force-pushed the channelz-test-direct-error branch from b6734d9 to 179ead7 Compare January 13, 2026 13:32
Three goroutines could outlive a call to ClientConn.close(). Add
mechanics to cancel them and wait for them to complete when closing a
client connection.

RELEASE NOTES:
- Closing a client connection will cancel all pending goroutines and
  block until they complete.

Signed-off-by: Tom Wieczorek <[email protected]>
@twz123 twz123 force-pushed the channelz-test-direct-error branch from 179ead7 to e8e0327 Compare January 13, 2026 13:45
@github-actions
Copy link

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.

@github-actions github-actions bot added stale and removed stale labels Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants