Skip to content

fix(cluster): synchronize shared-listener start-once to stop worker-listen race#1240

Merged
nickna merged 1 commit into
mainfrom
fix-cluster-listener-race
Jul 4, 2026
Merged

fix(cluster): synchronize shared-listener start-once to stop worker-listen race#1240
nickna merged 1 commit into
mainfrom
fix-cluster-listener-race

Conversation

@nickna

@nickna nickna commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the intermittent 60s CI timeout in the cluster port-sharing tests (RoundRobin_DistributesConnectionsAcrossWorkers, Fork_WorkersShareNetPort, Fork_WorkersShareHttpPort, Compiled mode). This is a real data race, not test flakiness — pre-existing on main.

It surfaced on the CI run for #1239 (inner-function fix), was root-caused there, but the fix landed on the branch after #1239 had already merged — so it never reached main. This PR lands it.

Root cause

SharedTcpListener.AddWorker / SharedHttpListener.AddWorker (Runtime/Types/SharedListenerRegistry.cs) started the underlying listener with an unsynchronized check-then-act:

if (!_started) { _started = true; _listener.Start(); StartAccepting(); }

Under SCHED_RR, both cluster workers call server.listen() on the same port from their own interpreter threads, and ConcurrentDictionary.GetOrAdd hands both the same SharedTcpListener. When the two AddWorker calls overlap:

  • both read _started == false, both set it true, and both call _listener.Start();
  • the second Bind on the same socket throws;
  • that exception propagates out of SharpTSNetServer.ListenAsClusterWorker before it fires the listening callback / process.send('ready');
  • the primary's readyCount never reaches 2, the connections are never made, and the module hangs to the 60s timeout.

Multi-core + CI load widens the race window, so it appeared intermittently, Compiled-mode only, and as a timeout (not an assertion failure).

Fix

Wrap the start-once transition in a lock — double-checked, publishing _started only after Start() succeeds (so a genuinely failed first Start can still be retried) — in both SharedTcpListener and SharedHttpListener, and guard Stop() with the same lock.

Verification

  • Reproduced on the pre-fix build: looping the three port-sharing tests, Fork_WorkersShareNetPort(mode: Compiled) hung 61s on the first iteration; the next passed in 7s.
  • Post-fix: 40/40 iterations of the three tests pass with no hang (plus a further reconfirmation loop on this branch's base).
  • Full ClusterModule / Net / Http suite: 130 / 0.

…isten race

Root-cause a flaky 60s CI timeout in the cluster port-sharing tests
(RoundRobin_DistributesConnectionsAcrossWorkers, Fork_WorkersShareNetPort,
Fork_WorkersShareHttpPort), Compiled mode. It was not test flakiness but a real
data race in SharedListenerRegistry.

SharedTcpListener.AddWorker / SharedHttpListener.AddWorker started the underlying
listener with an unsynchronized check-then-act:

    if (!_started) { _started = true; _listener.Start(); StartAccepting(); }

Under SCHED_RR both cluster workers call server.listen() on the same port from
their own interpreter threads, and ConcurrentDictionary.GetOrAdd hands both the
SAME SharedTcpListener. When the two AddWorker calls overlap, both pass the
`!_started` check and both call _listener.Start(); the second Bind on the same
socket throws. That exception propagates out of ListenAsClusterWorker before it
fires the `listening` callback / process.send('ready'), so the primary's
readyCount never reaches 2, the connections are never made, and the module hangs
to the 60s timeout. Multi-core + CI load widens the window, so it surfaced
intermittently and as a timeout (not an assertion failure).

Wrap the start-once transition in a lock (double-checked, publishing `_started`
only after Start() succeeds so a genuinely failed first Start can be retried) in
both shared listeners, and guard Stop() with the same lock.

Reproduced locally on the pre-fix build: Fork_WorkersShareNetPort(Compiled) hung
61s on the first loop iteration. Post-fix: 40/40 iterations of the three
port-sharing tests pass with no hang; full ClusterModule/Net/Http suite 130/0.
@nickna nickna merged commit fb7013a into main Jul 4, 2026
2 checks passed
@nickna nickna deleted the fix-cluster-listener-race branch July 4, 2026 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant