Skip to content

Bind the MessageQueue remote socket atomically - #42695

Merged
hanming-lu merged 1 commit into
mainfrom
oss-sync-4bccd1d184
Oct 6, 2026
Merged

hanming-lu merged 1 commit into
mainfrom
oss-sync-4bccd1d184

Conversation

@metamergebot

@metamergebot metamergebot commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

MessageQueue.__init__ sets up the remote XPUB socket in two steps: get_open_port() probes for a free port, then remote_socket.bind() claims it. Nothing holds the port in between. When several writers with remote readers are created concurrently on one host (for example, multi-node model-parallel groups that each build a broadcaster during initialization), two of them can be handed the same port, and the later one dies in __init__:

  File ".../sglang/srt/distributed/device_communicators/shm_broadcast.py", line 244, in __init__
    self.remote_socket.bind(address)
zmq.error.ZMQError: Address already in use (addr='tcp://[::1]:50003')

The failing rank takes its whole process group down with it. #29455 closed this window for the local socket; the remote socket still has it. #15903 proposed moving both sockets onto get_zmq_socket as a wider refactor, but it has been in conflict with main for months.

Modifications

  • shm_broadcast.py: replace the probe-then-bind pair for the remote socket with pyzmq's bind_to_random_port, which binds atomically and retries on EADDRINUSE. The window starts at the port get_open_port() returned and spans 32 ports, so SGLANG_PORT still anchors the range and the ephemeral fallback is unchanged. Exhausting the window raises ZMQBindError. IPV6 is still set before binding, and the exported handle carries the port that was actually bound.
  • The local-socket path is untouched. No new flag.

Accuracy Tests

Not applicable: only the way the remote broadcast socket picks its port at initialization changes.

Speed Tests and Profiling

Not applicable: initialization only, nothing on the request path.

Test plan

CPU-only script outside the repo that imports the real shm_broadcast and network modules, run against this branch and against main, for IPv4 and IPv6:

  • Race made deterministic: get_open_port is patched to return a port another socket already holds. main raises ZMQError(EADDRINUSE) from __init__; this branch binds another port inside the window.
  • Four writer processes started concurrently with the same starting port: on main three of the four fail with EADDRINUSE; on this branch all four bind distinct ports inside the window.
  • Writer plus remote reader round trip through the bound socket (wait_until_ready, enqueue, dequeue) still delivers.
  • Every port in the window occupied raises ZMQBindError.
  • ruff, ruff-format, isort and codespell at the versions pinned in .pre-commit-config.yaml pass on the changed file.

Checklist


CI States

Latest PR Test (Base): 🚫 Run #37401987293
Latest PR Test (Extra): ❌ Run #37402003034
Latest PR Test (AMD ROCm 10): ❌ Run #37401987257

When several MessageQueue writers with remote readers start at the same
time on one host, get_open_port() can hand two of them the same port: it
only probes for a free port, and the later remote_socket.bind() is a
separate step. The loser dies in __init__ with
"zmq.error.ZMQError: Address already in use", taking down the process
group it was initializing.

Replace the probe-then-bind pair with pyzmq's bind_to_random_port, which
binds atomically and retries on EADDRINUSE. The search window starts at
the probed port and spans 32 ports, so SGLANG_PORT still anchors the port
range; exhausting the window raises ZMQBindError. The IPv6 socket option
is still set before the bind, and the exported handle carries the port
that was actually bound. The local socket already binds this way.

Co-authored-by: charlotte12l <charlotte12l@users.noreply.github.com>
@metamergebot metamergebot added the run-ci CI: run the baseline test suite on this PR label Oct 6, 2026
@metamergebot

Copy link
Copy Markdown
Collaborator Author

This port clears our bar on dc56fa4b7f084476e7ec0576ee60733826da2842: every base-a and base-b job is green.
base-c and the accelerator suites are reported but not gated, because they run
on scarce hardware and upstream merges through them routinely.

gate 20✓✗2✂4 lint✓ build 3✓✗1✂3 A✓ B✓ C 0/8⏳ other 6✓✂6 · accel ✗amd,npu,xpu

@merrymercy @charlotte12l — could one of you take a look and merge it if you are happy? Nothing else is waiting on our side.

@hanming-lu
hanming-lu merged commit 6b737fd into main Oct 6, 2026
146 of 191 checks passed
@hanming-lu
hanming-lu deleted the oss-sync-4bccd1d184 branch October 6, 2026 22:51
cursor Bot pushed a commit to cipheraxat/sglang that referenced this pull request Oct 9, 2026
Co-authored-by: merrymercy <merrymercy@users.noreply.github.com>
Co-authored-by: charlotte12l <charlotte12l@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants