Skip to content

Fix flaky NettyClientAlpnTest by retrying on port-bind race conditions#7142

Merged
joviegas merged 4 commits into
masterfrom
joviegas/alpn-flaky-test-retry
Jul 14, 2026
Merged

Fix flaky NettyClientAlpnTest by retrying on port-bind race conditions#7142
joviegas merged 4 commits into
masterfrom
joviegas/alpn-flaky-test-retry

Conversation

@joviegas

@joviegas joviegas commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Motivation and Context

NettyClientAlpnTest intermittently fails with IOException: Failed to bind to 0.0.0.0:<port>.

Root cause: BaseMockServer.getUnusedPort() opens a ServerSocket(0) to get a free port number, then closes the socket before the port is actually used. Between that close and the later server.start() bind, the port is unowned and can be taken by another process on the CI host (example an outbound connection's ephemeral source port). When that happens, Jetty's bind fails and the test errors out, unrelated to any ALPN/HTTP2 code being tested.

Modifications

  • MockH2Server.java: connectors now bind to port 0, so the kernel assigns a free port atomically at bind time and the allocate-then-free race is gone. start() reads the actual ports back via connector.getLocalPort().
  • BaseMockServer.java: removed getUnusedPort() and the constructor that pre-allocated ports; it now only holds the port fields and URI helpers.
  • NettyClientAlpnTest.java: no functional change, tests stay plain @Test.

Testing

Verified by forcing a real port collision: retry fired for 3 attempts as configured, then passed once the port was free.

License

  • I confirm that this pull request can be released under the Apache 2 license

@joviegas joviegas requested a review from a team as a code owner July 14, 2026 14:34
@dagnir

dagnir commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Root cause: BaseMockServer.getUnusedPort() opens a ServerSocket(0) to get a free port number, then closes the socket before the port is actually used. Between that close and the later server.start() bind, the port is unowned and can be taken by another process on the CI host (example an outbound connection's ephemeral source port). When that happens, Jetty's bind fails and the test errors out, unrelated to any ALPN/HTTP2 code being tested.

Weird. Can we just fix the server so that it doesn't do this weird allocate then free dance? Forcing every test that uses BaseMockServer to retry doesn't seem very scalable

@joviegas

Copy link
Copy Markdown
Contributor Author

Root cause: BaseMockServer.getUnusedPort() opens a ServerSocket(0) to get a free port number, then closes the socket before the port is actually used. Between that close and the later server.start() bind, the port is unowned and can be taken by another process on the CI host (example an outbound connection's ephemeral source port). When that happens, Jetty's bind fails and the test errors out, unrelated to any ALPN/HTTP2 code being tested.

Weird. Can we just fix the server so that it doesn't do this weird allocate then free dance? Forcing every test that uses BaseMockServer to retry doesn't seem very scalable

Done, added the fix to get the port right before the start.
Initially I did that fix but moved to retry since this involved 1 file change and not touching the BaseMockServer.

@joviegas joviegas enabled auto-merge July 14, 2026 20:30
@joviegas joviegas added this pull request to the merge queue Jul 14, 2026
Merged via the queue into master with commit 38111dc Jul 14, 2026
12 of 13 checks passed
@github-actions

Copy link
Copy Markdown

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants