Second getFreePort bind-close-rebind race in streamable proxy tests
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Follow-up to #6034 / #6044. Same bug, second location, found while fixing the first.
Summary
pkg/transport/proxy/streamable/streamable_proxy_integration_test.go:25 has its own getFreePort with the identical bind-close-rebind (TOCTOU) shape that #6034 documents:
// getFreePort returns a free port by binding to port 0 and getting the assigned port
func getFreePort(t *testing.T) int {
t.Helper()
listener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
defer listener.Close() // <-- releases the port
return listener.Addr().(*net.TCPAddr).Port // <-- caller binds it later
}
The port is released before it is used, so anything else in the process can claim it in the gap. 8 call sites in that package, each feeding the port to NewHTTPProxy("localhost", port, …).
#6044 fixes only the test/integration/vmcp copy — deliberately, to keep that PR to one logical change. This tracks the second.
Why it is worth fixing rather than waiting for it to bite
#6044 quantified the mechanism with a standalone harness: 24 collisions in 31,773 rebind attempts (~0.08%) under contention, all bind: address already in use. That is exactly the rate that produces an occasional red check on an unrelated PR and gets waved through as "just CI".
An important nuance from that work, which applies here too: the race is narrower than it looks. Linux's ephemeral-port allocator actively avoids immediate reuse — 8 competing binders over a 50ms window produced 0 collisions in 300 attempts despite 74,630 competing binds; reproduction needed 64 victims plus 16 churners. So a long history of green runs is not evidence this copy is safe — it is evidence the window is usually not hit.
Suggested fix
Whether the clean fix from #6044 transfers depends on the API here, and that needs checking rather than assuming:
- #6044's approach worked because
vmcpserver.ServerConfig.Portaccepts0meaning "OS-assigned", and the server exposes the bound address viaAddress()afterReady(). Verify whetherNewHTTPProxy/ the streamable proxy has an equivalent — a port-0 path and a way to read the actual bound address back after start. - If it does, the fix is the same shape: pass
0, read the address back, deletegetFreePort. - If it does not, the alternative is to keep the probe listener open and hand it to the proxy, or to add a bound-address accessor. That is a slightly larger change to a test-facing constructor.
Either way, please leave a comment at the site explaining why a probe listener must not be reintroduced — #6044 does this, and it is the part that stops the pattern coming back.
Context
Third test-reliability issue from this cluster, alongside #6026 (optimizer search-quality spec — nondeterministic, no obvious fix) and #6040 (group e2e concurrent workload starts — cause still unidentified). This one and #6034 are the two with unambiguous mechanisms and small local fixes.
Generated with Claude Code
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in pkg/transport/proxy/streamable/streamable_proxy_integration_test.go at getFreePort and its eight NewHTTPProxy call sites. Inspect the streamable proxy startup API to determine whether port 0 and a bound-address accessor are available, then run the package integration tests. Done means removing the bind-close-rebind race and documenting why the probe listener must not be reintroduced.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- networking, testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100