Skip to content

fix: configure port ranges to avoid TOCTOU port contention - #2380

Merged
yuki-97 merged 12 commits into
mainfrom
terryk/rl-433-bind-port-to-avoid-toctou-issues
Jun 4, 2026
Merged

fix: configure port ranges to avoid TOCTOU port contention#2380
yuki-97 merged 12 commits into
mainfrom
terryk/rl-433-bind-port-to-avoid-toctou-issues

Conversation

@terrykong

@terrykong terrykong commented May 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Replace OS ephemeral port allocation (bind("", 0)) with dedicated port ranges to avoid TOCTOU race conditions where another process grabs a port between discovery and binding
  • NeMo RL master address uses ports 11001-15000, Gym servers use 15001-20000, vLLM engines use 20001+ with 100-port spacing per engine
  • Add _bind_socket_in_range() helper and propagate port_range_low/port_range_high through RayVirtualCluster, all algorithm setup functions, GenerationConfig, vLLM workers, and NeMo-Gym

Closes RL-433

Test plan

  • CI passes (lint, type checks)
  • Verify port allocation stays within configured ranges during multi-node GRPO/DPO/SFT runs
  • Verify no port contention failures in colocated vLLM + training scenarios

@terrykong
terrykong requested review from a team as code owners May 1, 2026 05:56
@copy-pr-bot

copy-pr-bot Bot commented May 1, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@terrykong
terrykong requested review from ananthsub and yfw May 1, 2026 05:58
@terrykong
terrykong requested a review from a team as a code owner May 1, 2026 06:11
@terrykong
terrykong force-pushed the terryk/rl-433-bind-port-to-avoid-toctou-issues branch from bb40af5 to ce47da4 Compare May 27, 2026 18:11
@terrykong terrykong linked an issue May 27, 2026 that may be closed by this pull request
Comment thread nemo_rl/environments/nemo_gym.py
Comment thread nemo_rl/environments/nemo_gym.py Outdated

@terrykong terrykong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining findings — PR #2380

3 items from the earlier team review that are not yet addressed.

Generated by Claude Code

Comment thread nemo_rl/distributed/virtual_cluster.py Outdated
Comment thread nemo_rl/models/generation/vllm/vllm_worker.py
terrykong added 11 commits June 4, 2026 00:57
Replace OS ephemeral port allocation (port 0) with dedicated port ranges
to avoid TOCTOU race conditions where another process grabs a port between
discovery and binding:
- NeMo RL master address: 11001-15000
- Gym servers: 15001-20000
- vLLM engines: 20001+ (100-port spacing per engine)

Add _bind_socket_in_range() for random port selection within a range, and
propagate port_range_low/port_range_high through RayVirtualCluster, all
algorithm setup functions, GenerationConfig, vLLM workers, and NeMo-Gym.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Test _bind_socket_in_range, _get_free_port_local, and
RayVirtualCluster port range parameter propagation.

Signed-off-by: Terry Kong <terryk@nvidia.com>
…yout

Move Ray head (GCS), client server, and worker gRPC ports out of the
Linux ephemeral range (32768-60999) to prevent TOCTOU collisions where
the kernel assigns an ephemeral source port in the window between
Ray's CheckPortFree probe and the actual bind.

Port layout (all below ephemeral range):
  9900-9901    Ray GCS / client server
  10002-11000  Ray worker gRPC
  11001-15000  NeMo RL HTTP servers / TCPStore
  15001-20000  NeMo Gym HTTP servers
  20001+       vLLM TP/DP rendezvous

Also add port_range_low/high to the grpo_math_1B exemplar YAML so
users can discover and override the default master-address port range.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Configure port_range_low/high (15001-20000) in the nemo_gym exemplar
config so Gym HTTP servers stay in their dedicated range, non-overlapping
with NeMo RL (11001-15000) and vLLM (20001+).

Signed-off-by: Terry Kong <terryk@nvidia.com>
Add port_range_low/high to standalone configs that don't inherit
from grpo_math_1B.yaml:
- distillation_math.yaml: generation port range (11001-15000)
- grpo_nanov3.yaml: generation port range (11001-15000) + Gym port range (15001-20000)

Signed-off-by: Terry Kong <terryk@nvidia.com>
…t constants

- Move master address port range from policy.generation to cluster config
  (master_port_range_low/high on ClusterConfig TypedDict) so SFT/DPO/RM
  algorithms that lack generation configs can set it.  Default: 25000-28000.
- Centralize all port range constants in virtual_cluster.py:
  generation (11001-15000), gym (15001-20000), master (25000-28000).
- Remove magic numbers from vllm_worker_async, sglang_worker, nemo_gym
  in favor of named constants.
- Fix nemo_gym head_server_port to use the Gym range instead of the
  master range.
- Remove generation_config.get("generation", {}) anti-pattern from
  dpo/rm/sft — they read from cluster_config directly now.
- Update exemplar YAMLs and ray.sub port layout comment.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Address review feedback from @yfw — declare port_range_low/high as
NotRequired fields on NemoGymConfig so the TypedDict matches usage.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Document where the defaults live (DEFAULT_GYM_PORT_RANGE_LOW/HIGH
in virtual_cluster.py) so readers don't have to trace the .get() calls.

Signed-off-by: Terry Kong <terryk@nvidia.com>
…ests

Move _VLLM_PORT_RANGE_LOW and _VLLM_PORTS_PER_ENGINE from a local
variable in vllm_worker.py to centralized constants in virtual_cluster.py
(DEFAULT_VLLM_PORT_RANGE_LOW, DEFAULT_VLLM_PORTS_PER_ENGINE) alongside
the other port range constants.

Add parametrized tests for the engine_index_on_node calculation covering
TP=1, TP=2, TP=4, TP=8, and the no-bundle-indices case.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
SO_REUSEADDR lets the probe bind to TIME_WAIT ports, but the eventual
caller (NCCL, vLLM, uvicorn) does not set SO_REUSEADDR and will fail
to bind.  Removing it ensures the probe only returns truly available
ports.

Signed-off-by: Terry Kong <terryk@nvidia.com>
@terrykong
terrykong force-pushed the terryk/rl-433-bind-port-to-avoid-toctou-issues branch from 3cc3e09 to b74b60f Compare June 4, 2026 07:59
@terrykong
terrykong requested a review from yuki-97 June 4, 2026 07:59
@terrykong terrykong added the CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) label Jun 4, 2026
@terrykong

Copy link
Copy Markdown
Collaborator Author

/ok to test b74b60f

Add port_range_low/high and master_port_range_low/high to the
reference configs so test_reference_configs_up_to_date passes.

Signed-off-by: Terry Kong <terryk@nvidia.com>
@terrykong

Copy link
Copy Markdown
Collaborator Author

/ok to test ba6fdb6

@yuki-97
yuki-97 merged commit 7b568f7 into main Jun 4, 2026
75 of 76 checks passed
@yuki-97
yuki-97 deleted the terryk/rl-433-bind-port-to-avoid-toctou-issues branch June 4, 2026 13:30
pengdurice pushed a commit to pengdurice/RL that referenced this pull request Jun 12, 2026
terrykong added a commit that referenced this pull request Jul 25, 2026
vLLM 0.25's RayExecutorV2 picks the torch.distributed TCPStore port with a
bind-probe (Step 3) but only binds it much later, in the rank-0 worker's
init_process_group. In between, Step 4 builds the broadcast MessageQueue;
when the engine spans nodes that queue needs a real TCP socket, so it calls
get_open_port() and binds and holds the result (shm_broadcast.py:
remote_subscribe_port = get_open_port(), then remote_socket.bind(...)).
Both searches start at VLLM_PORT, so the queue takes the very port the probe
just released and startup dies with EADDRINUSE (DeepSeek-V3 generation TP=32,
observed on port 7000). Engines that fit on one node bind an ipc:// socket
instead and never allocate a TCP port here, which is why only node-spanning
engines are affected.

Fix: patch _select_tcpstore_port to start its search at VLLM_PORT + 32, past
the queue's scan range. Both ports stay inside the engine's 100-port window
and therefore below the OS ephemeral floor (as low as 9000 on some nodes).
vLLM applies the same disjoint-window idea to co-located DP engines a few
lines below, seeding them from master_port + 100 + rank * 32.

Deliberately NOT fixed by leaving VLLM_PORT unset for these engines: that
sends vLLM to _get_open_port()'s s.bind(("", 0)) fallback, i.e. kernel-assigned
ephemeral ports, which is exactly the TOCTOU contention the reserved port
layout exists to prevent (#2380, #3103). The port assignment in
configure_worker is therefore unchanged from main.

The patch verifies its own result by reading the file back, so a patch that
silently fails to land is visible in worker logs instead of degrading to the
collision at runtime.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Jul 26, 2026
vLLM 0.25's RayExecutorV2 picks the torch.distributed TCPStore port with a
bind-probe (Step 3) but only binds it much later, in the rank-0 worker's
init_process_group. In between, Step 4 builds the broadcast MessageQueue;
when the engine spans nodes that queue needs a real TCP socket, so it calls
get_open_port() and binds and holds the result (shm_broadcast.py:
remote_subscribe_port = get_open_port(), then remote_socket.bind(...)).
Both searches start at VLLM_PORT, so the queue takes the very port the probe
just released and startup dies with EADDRINUSE (DeepSeek-V3 generation TP=32,
observed on port 7000). Engines that fit on one node bind an ipc:// socket
instead and never allocate a TCP port here, which is why only node-spanning
engines are affected.

Fix: patch _select_tcpstore_port to search from VLLM_PORT + 32, past the
queue's scan range. Both ports stay inside the engine's 100-port window and
therefore below the OS ephemeral floor (as low as 9000 on some nodes).

The offset must run *before* the local_dp_rank test, not inside it.
_select_tcpstore_port's disjoint-window branch reads as though it only serves
co-located DP engines, but ParallelConfig.__post_init__ takes its offline-SPMD
path for every engine NeMo-RL builds and assigns data_parallel_rank_local =
VLLM_DP_RANK_LOCAL (0 by default) and data_parallel_master_port =
VLLM_DP_MASTER_PORT (0 by default). A plain non-DP engine therefore arrives
with local_dp_rank=0, not None: the None branch is dead code, and the DP branch
searches from 0 + 100 + 0*32 = 100, fails all 32 attempts on the privileged
range, and falls through to get_open_port() -- straight back to VLLM_PORT, the
exact port the MessageQueue holds. An earlier revision of this commit put the
offset inside the None branch and was silently inert on hardware. See RL-1104.

Deliberately NOT fixed by leaving VLLM_PORT unset: that sends vLLM to
_get_open_port()'s s.bind(("", 0)) fallback, i.e. kernel-assigned ephemeral
ports, which is exactly the TOCTOU contention the reserved port layout exists
to prevent (#2380, #3103). The port assignment in configure_worker is therefore
unchanged from main.

tests/unit/models/generation/test_vllm_tcpstore_port.py pins the arithmetic
using the values vLLM actually passes (local_dp_rank=0, master_port=0) and the
real patch applied to a copy of the installed executor, so a fix on a dead
branch fails the suite instead of passing review. It also pins the anchor
snippet so an upstream rename cannot silently turn the patch into a no-op.
The patch additionally re-reads the file and warns if the marker is absent.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Jul 27, 2026
vLLM 0.25's RayExecutorV2 picks the torch.distributed TCPStore port with a
bind-probe (Step 3) but only binds it much later, in the rank-0 worker's
init_process_group. In between, Step 4 builds the broadcast MessageQueue;
when the engine spans nodes that queue needs a real TCP socket, so it calls
get_open_port() and binds and holds the result (shm_broadcast.py:
remote_subscribe_port = get_open_port(), then remote_socket.bind(...)).
Both searches start at VLLM_PORT, so the queue takes the very port the probe
just released and startup dies with EADDRINUSE (DeepSeek-V3 generation TP=32,
observed on port 7000). Engines that fit on one node bind an ipc:// socket
instead and never allocate a TCP port here, which is why only node-spanning
engines are affected.

Fix: patch _select_tcpstore_port to search from VLLM_PORT + 32, past the
queue's scan range. Both ports stay inside the engine's 100-port window and
therefore below the OS ephemeral floor (as low as 9000 on some nodes).

The offset must run *before* the local_dp_rank test, not inside it.
_select_tcpstore_port's disjoint-window branch reads as though it only serves
co-located DP engines, but ParallelConfig.__post_init__ takes its offline-SPMD
path for every engine NeMo-RL builds and assigns data_parallel_rank_local =
VLLM_DP_RANK_LOCAL (0 by default) and data_parallel_master_port =
VLLM_DP_MASTER_PORT (0 by default). A plain non-DP engine therefore arrives
with local_dp_rank=0, not None: the None branch is dead code, and the DP branch
searches from 0 + 100 + 0*32 = 100, fails all 32 attempts on the privileged
range, and falls through to get_open_port() -- straight back to VLLM_PORT, the
exact port the MessageQueue holds. An earlier revision of this commit put the
offset inside the None branch and was silently inert on hardware. See RL-1104.

Deliberately NOT fixed by leaving VLLM_PORT unset: that sends vLLM to
_get_open_port()'s s.bind(("", 0)) fallback, i.e. kernel-assigned ephemeral
ports, which is exactly the TOCTOU contention the reserved port layout exists
to prevent (#2380, #3103). The port assignment in configure_worker is therefore
unchanged from main.

tests/unit/models/generation/test_vllm_tcpstore_port.py pins the arithmetic
using the values vLLM actually passes (local_dp_rank=0, master_port=0) and the
real patch applied to a copy of the installed executor, so a fix on a dead
branch fails the suite instead of passing review. It also pins the anchor
snippet so an upstream rename cannot silently turn the patch into a no-op.
The patch additionally re-reads the file and warns if the marker is absent.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Jul 27, 2026
vLLM 0.25's MessageQueue picks the port for its remote (TCP) socket with
get_open_port(), which binds a probe socket, releases it, and returns the
number; ZMQ binds it for real several statements later. That window is a
TOCTOU race, and on a non-driver node it is lost reliably rather than
occasionally.

Every RayWorkerProc on a non-driver node takes n_local_reader=0
(ray_executor_v2.py::_init_message_queues), so every one of them needs a
real TCP port and they all scan from the same VLLM_PORT -- 7000 for a
node-spanning engine. _init_message_queues runs immediately after
init_device(), whose process-group setup is a collective barrier, so all
workers on the node reach the probe within microseconds of each other,
all see the same port free, and all but one die with:

    zmq.error.ZMQError: Address already in use (addr='tcp://10.65.1.9:7000')

Workers on the driver node take n_local_reader=1 and use an ipc:// socket,
which is why only engines spanning >= 2 nodes are affected -- and why no
nightly test catches it, since none runs an engine whose
tensor_parallel_size * pipeline_parallel_size exceeds cluster.gpus_per_node.

Fix the race at the bind rather than the probe: retry, advancing past the
port that was lost. This terminates because a port a peer already holds
with ZMQ is visible to the next probe. Ports stay anchored at VLLM_PORT
rather than falling back to kernel-ephemeral ones, which is the contention
the reserved sub-ephemeral band exists to prevent (#2380, #3103). Patching
the bind rather than handing each worker a private start port also covers
every other MessageQueue with a remote reader, including the executor's own
rpc_broadcast_mq.

The code is byte-identical on vLLM main; this is an upstream bug, so the
same change is worth proposing there.

Verified against the real vLLM 0.25.1 MessageQueue with concurrent
processes: unpatched, 6/8 and 30/32 workers die (3/3 runs each); patched,
8/8 and 32/32 bind distinct ports inside the reserved band.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Jul 30, 2026
vLLM 0.25's RayExecutorV2 picks the torch.distributed TCPStore port with a
bind-probe (Step 3) but only binds it much later, in the rank-0 worker's
init_process_group. In between, Step 4 builds the broadcast MessageQueue;
when the engine spans nodes that queue needs a real TCP socket, so it calls
get_open_port() and binds and holds the result (shm_broadcast.py:
remote_subscribe_port = get_open_port(), then remote_socket.bind(...)).
Both searches start at VLLM_PORT, so the queue takes the very port the probe
just released and startup dies with EADDRINUSE (DeepSeek-V3 generation TP=32,
observed on port 7000). Engines that fit on one node bind an ipc:// socket
instead and never allocate a TCP port here, which is why only node-spanning
engines are affected.

Fix: patch _select_tcpstore_port to search from VLLM_PORT + 32, past the
queue's scan range. Both ports stay inside the engine's 100-port window and
therefore below the OS ephemeral floor (as low as 9000 on some nodes).

The offset must run *before* the local_dp_rank test, not inside it.
_select_tcpstore_port's disjoint-window branch reads as though it only serves
co-located DP engines, but ParallelConfig.__post_init__ takes its offline-SPMD
path for every engine NeMo-RL builds and assigns data_parallel_rank_local =
VLLM_DP_RANK_LOCAL (0 by default) and data_parallel_master_port =
VLLM_DP_MASTER_PORT (0 by default). A plain non-DP engine therefore arrives
with local_dp_rank=0, not None: the None branch is dead code, and the DP branch
searches from 0 + 100 + 0*32 = 100, fails all 32 attempts on the privileged
range, and falls through to get_open_port() -- straight back to VLLM_PORT, the
exact port the MessageQueue holds. An earlier revision of this commit put the
offset inside the None branch and was silently inert on hardware. See RL-1104.

Deliberately NOT fixed by leaving VLLM_PORT unset: that sends vLLM to
_get_open_port()'s s.bind(("", 0)) fallback, i.e. kernel-assigned ephemeral
ports, which is exactly the TOCTOU contention the reserved port layout exists
to prevent (#2380, #3103). The port assignment in configure_worker is therefore
unchanged from main.

tests/unit/models/generation/test_vllm_tcpstore_port.py pins the arithmetic
using the values vLLM actually passes (local_dp_rank=0, master_port=0) and the
real patch applied to a copy of the installed executor, so a fix on a dead
branch fails the suite instead of passing review. It also pins the anchor
snippet so an upstream rename cannot silently turn the patch into a no-op.
The patch additionally re-reads the file and warns if the marker is absent.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Jul 30, 2026
vLLM 0.25's MessageQueue picks the port for its remote (TCP) socket with
get_open_port(), which binds a probe socket, releases it, and returns the
number; ZMQ binds it for real several statements later. That window is a
TOCTOU race, and on a non-driver node it is lost reliably rather than
occasionally.

Every RayWorkerProc on a non-driver node takes n_local_reader=0
(ray_executor_v2.py::_init_message_queues), so every one of them needs a
real TCP port and they all scan from the same VLLM_PORT -- 7000 for a
node-spanning engine. _init_message_queues runs immediately after
init_device(), whose process-group setup is a collective barrier, so all
workers on the node reach the probe within microseconds of each other,
all see the same port free, and all but one die with:

    zmq.error.ZMQError: Address already in use (addr='tcp://10.65.1.9:7000')

Workers on the driver node take n_local_reader=1 and use an ipc:// socket,
which is why only engines spanning >= 2 nodes are affected -- and why no
nightly test catches it, since none runs an engine whose
tensor_parallel_size * pipeline_parallel_size exceeds cluster.gpus_per_node.

Fix the race at the bind rather than the probe: retry, advancing past the
port that was lost. This terminates because a port a peer already holds
with ZMQ is visible to the next probe. Ports stay anchored at VLLM_PORT
rather than falling back to kernel-ephemeral ones, which is the contention
the reserved sub-ephemeral band exists to prevent (#2380, #3103). Patching
the bind rather than handing each worker a private start port also covers
every other MessageQueue with a remote reader, including the executor's own
rpc_broadcast_mq.

The code is byte-identical on vLLM main; this is an upstream bug, so the
same change is worth proposing there.

Verified against the real vLLM 0.25.1 MessageQueue with concurrent
processes: unpatched, 6/8 and 30/32 workers die (3/3 runs each); patched,
8/8 and 32/32 bind distinct ports inside the reserved band.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants