Fix flaky TLS test by waiting for dial outcome before closing connection

[bcleenders]

PRs #564 and #567 introduced errSignalingConn to surface TLS errors (especially TLS 1.3 post-handshake alerts) to BlockingDial callers. This caused a flaky test failure (reported in: #564 (comment)):

TestBrokenTLS_RequireClientCertButNonePresented:
expecting a TLS certificate error, got: read tcp ...: use of closed
network connection

The race works as follows:

1. With TLS 1.3, the server rejects the connection after the handshake completes (e.g. missing client cert) by sending a TLS alert, then closing the connection.
2. Simultaneously, the client's Write fails (broken pipe) because the server already closed its side.
3. grpc-go's NewHTTP2Client returns the Write error and calls t.Close() to tear down the transport, which calls errSignalingConn.Close() -> c.Conn.Close(), closing the connection.
4. The reader goroutine's pending Read (which should have returned the TLS alert (e.g. "certificate required")) instead returns use of closed network connection because the connection was closed underneath it.
5. writeResult surfaces the "closed network connection" error to the caller instead of the meaningful TLS alert.

The fix: delay Close on the errSignalingConn until the outcome of the dial is known, with a 50ms timeout. This gives the reader a chance to surface the TLS alert.

Demonstrating the fix

Let's run the test a few thousand times, and do it in parallel so we stress the machine a bit. The failure only happens on machines with some load.

Without the fix

alasia :: github/fullstorydev/grpcurl ‹master› » go test -c -o /tmp/grpcurl.test . && GOMAXPROCS=2 $HOME/go/bin/stress -p 16 -count 5000 -ignore 'assign requested address|context deadline exceeded' /tmp/grpcurl.test -test.run 'TestBrokenTLS'

/var/folders/7l/0tdjxwg912j9gqh446yk3ll40000gn/T/go-stress-20260724T113423-1710148011
--- FAIL: TestBrokenTLS_RequireClientCertButNonePresented (0.00s)
    tls_settings_test.go:332: expecting a TLS certificate error, got: read tcp 127.0.0.1:52850->127.0.0.1:52848: use of closed network connection
FAIL

ERROR: exit status 1

[...]

5s: 1580 runs so far, 5 failures (0.32%), 13 active

(failure rate would likely be higher on machines with more load)

With the fix

alasia :: github/fullstorydev/grpcurl ‹surface-post-dial-conn-errors› » go test -c -o /tmp/grpcurl.test . && GOMAXPROCS=2 $HOME/go/bin/stress -p 16 -count 5000 -ignore 'assign requested address|context deadline exceeded' /tmp/grpcurl.test -test.run 'TestBrokenTLS'
5s: 1594 runs so far, 0 failures, 16 active
10s: 3386 runs so far, 0 failures, 16 active
15s: 4999 runs so far, 0 failures, 1 active
15s: 5000 runs total, 0 failures

Why 50ms?

It's not super important; if the TLS alert takes longer than this threshold, users just get a less descriptive error ("closed connection" instead of the TLS error).

50ms is long enough that I assume go routines would propagate the error (it doesn't need to be longer than a network roundtrip, for example), while short enough that it seems unnoticeable when you do hit it (since you'd wait much longer than 50ms for the connection to be tried and eventually fail).

---

Note

Medium Risk
Touches all TLS dials via BlockingDial with a bounded teardown delay; behavior change is limited to error reporting and close timing on failing connections.

Overview
Fixes a race where grpc-go could close the TLS-wrapped connection before a post-handshake alert was read, so BlockingDial sometimes surfaced "use of closed network connection" instead of the real TLS error (e.g. missing client cert under TLS 1.3).

BlockingDial now signals when the dial outcome is known via a dialCompleted channel (closed from writeResult and on function exit). That signal is passed through errSignalingCreds into errSignalingConn, whose Close waits until the dial completes or up to 50ms before delegating to the underlying connection—giving the reader goroutine time to report the TLS alert.

^{Reviewed by Cursor Bugbot for commit ff0d65c. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 896a4df. Configure here.}

[bcleenders]

The build failure is just a refusal to build;

Error: Refusing to check out fork pull request code from a 'pull_request_target' workflow. This workflow runs with the base repository's GITHUB_TOKEN, secrets, default-branch cache scope, and runner access. Fetching and executing a fork's code in that trusted context commonly leads to "pwn request" vulnerabilities.

[dragonsinth]

I originally debated on whether a live-code change was justified to handle a test flake (vs. adding another allowed exception to the test assertion), and I guess I came down on the side of better error message even in a live run. CC: @jhump