[Bugfix][V1] Fix TOCTOU race causing intermittent EADDRINUSE on multi-API-server DP startup

[vadiklyutiy]

Problem

Running multi-node DP on 2×(4×GB300) (data_parallel_size=8, data_parallel_size_local=4, api_server_count=8, NVFP4 MoE), 5 of 79 startups failed with the same error from ApiServer_0:

(ApiServer_0 pid=...) zmq.error.ZMQError: Address already in use (addr='tcp://<master>:39381')
...
RuntimeError: Process ApiServer_0 (PID: ...) died with exit code 1

Root cause

The rank-0 parent pre-picks ephemeral TCP ports via bind(("", 0)) → close() in get_open_port() and only later spawns each API server child, which does the real ZMQ bind — typically 5–25 s later after model config and scheduler setup. In that window any other process on the host can claim the freed port. The most likely culprit is vLLM itself: it allocates ~30+ TCP ports in the same startup burst (per-API-server inputs/outputs, DP-master pool, handshake, coordinator, stats), and the kernel can hand the same ephemeral port number back to two different get_open_port() calls inside the same parent before either child has had a chance to bind. Stale processes from prior crashed runs amplify the risk (the failing log also shows leaked-semaphore / leaked-shared-memory warnings at shutdown).

Fix

Apply the same bind-in-child + report-back-via-Pipe pattern that DPCoordinator already uses. The parent emits tcp://host:0 placeholders; each ApiServer_i binds its ROUTER/PULL with port 0 (kernel atomically picks a free port), reads the bound endpoints via getsockopt(zmq.LAST_ENDPOINT), and reports them to the parent over a per-child multiprocessing.Pipe. The parent collects the actual addresses before the engine startup handshake propagates them to every engine on every DP node. No protocol changes; opt-in everywhere; IPC paths and the single-client (engines-managed-by-this-client) path are unchanged.

Tests

Unit test

tests/entrypoints/test_api_server_process_manager.py:

• End-to-end happy path with defer_addresses=True — 4 children, kernel-picked ports, addresses gathered and verified distinct & well-formed.
• gather_actual_addresses() rejects use without defer_addresses=True.
• Child exits before reporting → clear RuntimeError, no hang.

Stress testing

On a common case the bug fires ~1/100 runs (DP=8, DP-local=4, 2 nodes) — too rare for reliable verification.

To make it reproducible I ran each compute node with a small "port squatter" that pre-binds every port in /proc/sys/net/ipv4/ip_local_port_range except a small window at the top, leaving _get_open_port() only ~100 ports to choose from. Under that pressure the TOCTOU race fires almost every run.

Squatter (started in background on each node before vllm serve):

import os, socket, sys, time, signal
free_top = int(sys.argv[1])
with open("/proc/sys/net/ipv4/ip_local_port_range") as f:
    lo, hi = map(int, f.read().split())
squat_hi = hi - free_top + 1
held = []
for p in range(lo, squat_hi):
    try:
        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        s.bind(("0.0.0.0", p)); s.listen(1)
        held.append(s)
    except OSError:
        pass
print(f"[squatter] holding {len(held)} ports", flush=True)
signal.signal(signal.SIGTERM, lambda *a: sys.exit(0))
while True: time.sleep(3600)

Driver per node:

ulimit -n 65535
python3 squatter.py 100 &        # leave only 100 ports free
exec vllm serve <args>

Results (same config, 2 nodes, DP=8, DP-local=4, squatter at free_top=100):

build	passed	failed
main	1	27
this PR	50	0

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[gemini-code-assist]

Code Review

This pull request implements a deferred ZMQ port assignment mechanism where child processes bind to kernel-assigned ports and report them back to the parent via pipes. This avoids potential race conditions and port conflicts during initialization. The changes include updates to the APIServerProcessManager to manage these pipes and gather addresses, as well as integration into the CLI and engine client. Review feedback highlights the need for explicit resource management of the multiprocessing pipes, specifically recommending that they be closed immediately after use and during the manager's shutdown to prevent file descriptor leaks.

[vadiklyutiy]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements a mechanism for API servers to defer ZMQ port assignment to the kernel, allowing them to bind to available ports dynamically and report these addresses back to the parent process via pipes. Key changes include the addition of the gather_actual_addresses method to APIServerProcessManager, logic in CoreClient to report bound endpoints, and integration into the CLI server startup process. Feedback points out a type hint inconsistency in client_addresses that requires a type ignore, suggesting a future refactor to dict[str, Any] to restore type safety.

[gemini-code-assist]

The use of # type: ignore[assignment] here is necessary because client_addresses is currently type-hinted as dict[str, str] | None in the __init__ signature, but it now contains a multiprocessing.connection.Connection object. While fixing the signature is outside the scope of this diff hunk, the current annotation is technically incorrect and forces this suppression. Consider updating the type hint of client_addresses to dict[str, Any] | None in a future refactor to restore type safety.

[vadiklyutiy]

I made stress testing and added corresponding section in PR description.

Before run, grabbed free ports and left ~100 free. Results:

build	passed	failed
main	1	27
this PR	50	0

[vadiklyutiy]

Just found that #40596 does the same

[vadiklyutiy]

and #41983 do the same

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @vadiklyutiy.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[robertgshaw2-redhat]

Im wondering, should we just update all cases to launch like this rather than maintaining multiple special cases

[robertgshaw2-redhat]

where are these actually used in the main process?

I don't quite follow where wait_for_completion_or_failure actually runs these

[robertgshaw2-redhat]

I see, its because this is part of the launch_core_engines context manager and these results are used after the yield in the context manager

this seems very brittle and hard to follow

[robertgshaw2-redhat]

IIUC, the APIServerProcessManager is not used anywhere else besides here. Why do we need to support multiple cases?

[njhill]

Agree we could always do this and avoid the arg.

It's also used in tests but AFAICT it shouldn't affect those, or if it does they could be adjusted accordingly.

[robertgshaw2-redhat]

Thank you for this PR, this fix is long overdue and the fix is solid

In terms of the details of this PR, we are maintaining an "old way" and a "new way". I think this makes the code hard to follow. Why can't we just get rid of the "old way"?

• APIServerProcessManager is only used for multi-api server (https://github.com/search?q=repo%3Avllm-project%2Fvllm+APIServerProcessManager&type=code), we should only have once case

• launch_core_engines is only used in 2 places (https://github.com/search?q=repo%3Avllm-project%2Fvllm%20launch_core_engines&type=code) --- this should probably not be a context manager and I think we can just use the same mechanism in both cases

[njhill]

I agree this fix and PR review is long overdue, really sorry I am not keeping up with my backlog lately.

@robertgshaw2-redhat I agree the way that a context manager is used here is a bit opaque / hard to follow.

It's organized this way so that the engine process management and api server process management can be encapsulated in their own classes, while also allowing the startups / engine handshake to be interleaved in such a way that we parallelize things as much as possible and minimize the startup critical path time (so that we don't wait for the engines to be started before the api servers finish starting up). There's metatdata that needs to be wired in both directions.

The context manager allows this to be done "cleanly" in some respects but we can hopefully think of a better design.

[njhill]

Im wondering, should we just update all cases to launch like this rather than maintaining multiple special cases

One difference that the current abstractions cover is whether you are using vllm serve with possibly multiple API server procs, or AsyncLLM directly. The latter case encapsulates the core engine proc handling with AsyncLLM so that it can be used standalone (there are no api servers involved in this case), whereas the former wires things up externally.

[ehfd]

Fix for the above issue must be cherry-picked to v0.22.0 too. @khluu @mgoin

[vadiklyutiy]

@ehfd

• do we need for reproduction --hf-overrides ?
• --max-model-len 1010000 looks a bit strange. Is it correct value?

[ehfd]

I think it is reproducible with just 262144 context size and without the override.

The override and the context size is based on Qwen's documentation.

[vadiklyutiy]

@ehfd fix - #43768

[ehfd]

#43768 Fixes the issue completely. Thank you!