Conversation
…t server Replace the random-port-plus-check approach with TcpListener(0), which asks the OS to assign a free port. The TOCTOU window (between TcpListener.Stop() and Kestrel's bind) is microseconds, far more reliable than the previous approach that randomly selected a port from 10000-65000 and then checked IPGlobalProperties.GetActiveTcpListeners(). Also removes the Windows-CI skip workaround from testMultipartFormDataBodySize, since the root cause (flaky port selection) is now fixed. Also fixes Dispose() to properly await StopAsync instead of fire-and-forget. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4 tasks
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.
🤖 This is an automated pull request from Repo Assist, an AI assistant for this repository.
Problem
The
startHttpLocalServer()helper inHttp.fsselected a free port by:IPGlobalProperties.GetActiveTcpListeners()in a loop until no conflictThis approach has a TOCTOU race (time-of-check to time-of-use): another process can grab the port between the check and Kestrel's bind. On busy Windows CI agents (many parallel processes, ephemeral ports recycled rapidly) this caused intermittent failures —
1 Failed, 2889 Passed— visible in the Windows CI job for PR #1717.The
testMultipartFormDataBodySizetests already carried a Windows-CI-specificAssert.Ignoreworkaround for this exact issue.Fix
Replace the random-port-then-check logic with
TcpListener(IPAddress.Loopback, 0):The OS assigns a free port (via the kernel, atomically). The window between
Stop()and Kestrel'sBind()is microseconds — negligible in practice and far better than the previous approach.Also removes the now-unnecessary Windows-CI skip workaround from
testMultipartFormDataBodySize, and fixesDispose()to properly awaitStopAsyncinstead of fire-and-forget.Test Status
dotnet test tests/FSharp.Data.Core.Tests/ --filter Http— 136 passed, 0 failed, 0 skipped (multipart tests now run on all platforms without the skip workaround)dotnet test tests/FSharp.Data.Core.Tests/— 2896 passed, 0 failed