You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 portfuncgetFreePort(t*testing.T) int {
t.Helper()
listener, err:=net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
deferlistener.Close() // <-- releases the portreturnlistener.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:
Bind the vMCP test server's port directly #6044's approach worked because vmcpserver.ServerConfig.Port accepts 0 meaning "OS-assigned", and the server exposes the bound address via Address() after Ready(). Verify whether NewHTTPProxy / 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, delete getFreePort.
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.
Follow-up to #6034 / #6044. Same bug, second location, found while fixing the first.
Summary
pkg/transport/proxy/streamable/streamable_proxy_integration_test.go:25has its owngetFreePortwith the identical bind-close-rebind (TOCTOU) shape that #6034 documents: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/vmcpcopy — 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:
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.0, read the address back, deletegetFreePort.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