fix: Eliminate port allocation race condition in all affected test files - #1528
Closed
maxisbey wants to merge 2 commits into
Closed
fix: Eliminate port allocation race condition in all affected test files#1528maxisbey wants to merge 2 commits into
maxisbey wants to merge 2 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Fixes a TOCTOU (Time-of-Check-Time-of-Use) race condition where multiple pytest-xdist workers would attempt to bind to the same port, causing tests to fail with "address already in use" errors or connect to the wrong server. ## Root Cause The server_port() fixture would: 1. Bind to port 0 to get a free port 2. Immediately close the socket, freeing the port 3. Return the port number When tests ran in parallel with pytest-xdist, multiple workers could get the same "free" port between steps 2 and 3, leading to conflicts. ## Solution Implement worker-specific port ranges using pytest-xdist's worker_id: - Each worker gets a dedicated range from 40000-49999 - Port ranges are calculated based on PYTEST_XDIST_WORKER_COUNT - Guarantees no overlap between workers ## Changes - Add parse_worker_index() to extract worker number from worker_id - Add calculate_port_range() to compute non-overlapping port ranges - Refactor get_worker_specific_port() to use the pure functions above - Update server_port fixture to use worker-specific ports - Add comprehensive unit tests (28 tests) covering all edge cases Tests now pass reliably with 14 workers in parallel.
Extended the fix from test_integration.py to all test files that suffered
from the same TOCTOU (Time-of-Check-Time-of-Use) race condition. When
running with pytest-xdist parallel workers, multiple workers would get
the same "free" port from `socket.bind(("127.0.0.1", 0))`, leading to:
- "address already in use" errors
- Tests connecting to wrong servers
Solution:
- Updated 7 test files to use `get_worker_specific_port(worker_id)`:
* tests/server/test_streamable_http_security.py
* tests/server/test_sse_security.py
* tests/shared/test_streamable_http.py (3 fixtures)
* tests/shared/test_sse.py
* tests/shared/test_ws.py
* tests/client/test_http_unicode.py
* tests/client/test_notification_response.py
- Fixed socket cleanup issue in tests/test_test_helpers.py
Each worker now gets an exclusive port range from 40000-59999, eliminating
the race condition across all test files that start servers.
Testing:
- All 703 tests pass with pytest-xdist parallel execution
- Port allocation now consistent across all parallel workers
maxisbey
marked this pull request as draft
October 28, 2025 23:39
Copy link
Copy Markdown
Contributor
Author
|
actually, I don't like this approach anymore. It's a bit late at night so I'm going to leave it as draft for now, but some other options I was thinking are:
Although none of them really are clean IMO. So going to leave this for now |
Sorry, something went wrong.
This was referenced Mar 10, 2026
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
2 participants
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 PR extends the port allocation race condition fix to all test files affected by the same TOCTOU issue, eliminating intermittent test failures when running with pytest-xdist parallel workers.
Motivation and Context
When running tests with pytest-xdist parallel workers, multiple workers would receive the same "free" port from socket.bind(("127.0.0.1", 0)), causing a TOCTOU (Time-of-Check-Time-of-Use) race condition. This manifested as:
This builds on the initial fix in tests/server/fastmcp/test_integration.py by applying the same solution to 7 additional test files that had the identical issue.
How Has This Been Tested?
Each pytest-xdist worker now receives an exclusive port range from 40000-59999 based on its worker ID, eliminating port conflicts entirely.
Breaking Changes
None. This is an internal test infrastructure change with no impact on the public API or user-facing functionality.
Types of changes
Checklist
Additional context
Files modified:
Implementation details:
All affected fixtures now use get_worker_specific_port(worker_id) instead of the race-prone socket.bind(("127.0.0.1", 0)) pattern. The helper function allocates worker-specific port ranges ensuring no overlap between parallel test workers.