Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
closeAfterTimeoutcan close a connection after a successful TLS handshake, even though its timeout has not expired. This is a scheduling/logic race, not a Go data race and not specific to Docker, proxies, DNS, or a VM clock.The helper is unchanged in v0.5.14, v0.5.19 and current main/v0.5.21 (
a6c0214c35891524aee4c76ad0f6cb11bbe87f5f). We encountered it through httpx/nuclei builds using the first two versions; the reproducer below uses only the Go standard library and the unchanged helper on native macOS/arm64.Why it happens
The returned completion function calls both
handshakeDoneCancel()and the timeout context'scancel(). If the watchdog reaches itsselectafter both calls, bothDone()channels are ready. Selectingctx.Done()currently callsClose()unconditionally, even whenctx.Err()iscontext.Canceled, notcontext.DeadlineExceeded.The regular TLS, uTLS, ZTLS and ZTLS fallback paths share this helper. In a successful path the connection can therefore be handed back to the caller and then closed by its own watchdog. This can cause subsequent I/O to fail. Concurrency or scheduler delay can expose the interleaving; no actual timeout is required.
The existing cancellation test sleeps before cancelling, allowing the watchdog to start waiting on
select. The new test deliberately cancels immediately, covers repeated cancellation, and advances virtual time past the original deadline without real sleeps.Fix
Check that the timeout context actually expired before closing its resources. Keep the existing ownership and deadline behavior; this does not change TLS verification, retries, timeout values, or dependencies. The separate watchdog-overhead work in #554 is outside this fix; #552 concerned cleanup on handshake failure rather than closure after success.
Reproduction and validation
Environment:
go version go1.27.1 darwin/arm64.On unmodified main with the new test:
The number is scheduling-dependent, not an expected failure rate for real scans. Three isolated runs of the exact helper plus the same regression test (standard library only) produced 121, 119 and 133 erroneous
Closecalls, out of 256 cancelled watchdogs per run.With the fix:
The focused tests include the existing deadline/cancellation checks and loopback TLS/ZTLS handshake-failure checks. Full unfiltered
go test ./...is not claimed: several existing tests contact external hosts. No public targets or product infrastructure are required for this regression test. The test usestesting/synctest, available with the repository's Go 1.25 minimum.