Fix flaky HttpListener tests by using dynamic port allocation - #10939
Conversation
… of hardcoded ports Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes flakiness in AndroidMessageHandlerTests caused by HttpListenerException: Address already in use by removing hardcoded listener ports and allocating ports dynamically at runtime.
Changes:
- Added a
GetAvailablePort()helper that usesTcpListeneron port0to obtain an OS-assigned free port. - Updated
DoesNotDisposeContentStreamandHttpContentStreamIsRewoundAfterCancellationto use dynamically selected ports instead of hardcoded47663/47664. - Added
using System.Net.Sockets;forTcpListener.
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict: ✅ LGTM
Found 1 suggestion (0 errors, 0 warnings).
- 💡 Resource management:
TcpListenercould useusingfor defense-in-depth (AndroidMessageHandlerTests.cs:106)
👍 Clean, focused fix. The GetAvailablePort() pattern (bind port 0, read assigned port, release) is the standard way to avoid hardcoded-port flakiness in tests. The TOCTOU window between releasing and rebinding is inherent to this technique but negligible in practice.
CI note: Xamarin.Android-PR is failing but this is pre-existing — all recent runs on that pipeline are failing with batchedCI reason. The public dotnet-android Linux build passed; Mac/Windows builds are still in progress.
Review generated by android-reviewer from review guidelines.
- Changed IPAddress.Loopback to IPAddress.Any to match the http://+:{port}/
prefix which binds all interfaces
- Added using statement for defense-in-depth resource cleanup
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Fixes the flaky
HttpContentStreamIsRewoundAfterCancellationtest failure caused bySystem.Net.HttpListenerException: Address already in usewhen hardcoded port 47664 is unavailable.Root Cause
The tests
DoesNotDisposeContentStreamandHttpContentStreamIsRewoundAfterCancellationused hardcoded ports (47663 and 47664) for theirHttpListenerinstances. When these ports are already in use by another process or a previous test run that didn't clean up properly, the tests fail with "Address already in use".Changes
GetAvailablePort()helper method that usesTcpListenerwith port 0 to dynamically allocate an available port from the OSHttpContentStreamIsRewoundAfterCancellationto use dynamic port instead of hardcoded 47664DoesNotDisposeContentStreamto use dynamic port instead of hardcoded 47663 (same vulnerability)System.Net.Socketsusing directiveSecurity Summary
No security vulnerabilities introduced or discovered. The change only replaces hardcoded port numbers with dynamically allocated ones in test code.