Skip to content

fix(cluster-tool): scope the bind-registry port lock to the registry it guards - #76

Open
valthon wants to merge 1 commit into
masterfrom
fix/test-gate-port-lock-scoping
Open

fix(cluster-tool): scope the bind-registry port lock to the registry it guards#76
valthon wants to merge 1 commit into
masterfrom
fix/test-gate-port-lock-scoping

Conversation

@valthon

@valthon valthon commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Important

Merge AFTER #57. The jest testTimeout bump that master needs for a green
suite now lives in #57, so this PR's CI is red on its own until that lands — see
"Test status" below. Rebase onto master once #57 is in and this goes green.

Split out of #57 at review request ("Incredibly dangerous, please split to a
separate PR" / "Split out the port lock changes into another PR"). One commit,
three files, no functional change to any cluster/flow code path.

The lock is keyed by the registry it guards, and lives outside it

BindConfigProvider.portLockPath() now hashes the normalized registry path (so two
spellings of one directory resolve to one lock) and places the lock file in the OS temp
dir rather than inside the registry directory.

Real runs are unchanged. With WIRE_BIND_REGISTRY_PATH unset, every process resolves
the same default registry, hence the same lock — the host-global cross-process guarantee
holds exactly as before, and that is what keeps concurrent clusters from drawing the same
port.

A caller that sandboxes the registry (every jest suite does, in tests/jest.setup.ts)
hashes to its own lock. That is correct rather than a relaxation: disjoint registries
cannot collide on a port, so serializing them protects nothing — it only produced false
contention.

Two failure modes this closes:

  • Fixed tmpdir name → every port-drawing test across all 8 jest projects queued on one
    global lock while each already held a private registry, blowing FileLockOptions'
    retry budget (Lock file is already being held).
  • Lock inside the registry dir → a sandboxing suite deletes that dir at teardown while
    proper-lockfile's mtime-refresh timer still holds the lock; its onCompromised hook
    throws ENOENT ... wire-cluster-ports.lock.lock. Hashing the path keeps the scoping
    while putting the file somewhere no fixture owns.

Stale window raised above the critical section

FileLockOptions.stale 10s → 30s. The section it guards is a solana dynamic-range
pick that TCP+UDP-probes a 64-port window — ~15s on a loaded host. At 10s a live holder
was stolen mid-section, which is precisely how two clusters end up drawing the same port
and UDP-double-binding.

Test status — red until #57 merges

Correction to an earlier revision of this description. It previously claimed the
ENOENT ... wire-cluster-ports.lock.lock failures were a second defect that this PR
fixes. That was wrong, and the corrected causation is worth recording:

Those ENOENTs are a consequence of the 30s jest ceiling, not an independent bug. A
test killed at the ceiling dies inside withFileLock; jest tears the suite down; the
fixture removes its temp registry dir while proper-lockfile's refresh timer still holds
the lock; onCompromised then throws. One defect, two symptoms.

Measured on this host, on master + the collateral work:

Tree Result
timeout bump only, lock files untouched 220/220 suites, 1726/1726 tests — green
lock fix only, 30s ceiling red (~86 Exceeded timeout of 30000 ms)

So the timeout is what the BASELINE needs, independent of either PR's content — which is
why it belongs with #57 rather than here. This PR's own value is the concurrency
correctness above: the lock's scope matching the state it guards, and a stale window that
cannot steal a live holder. Those are real and unrelated to the ceiling; they just are not
what turns the suite green.

Verification

Based on origin/master (78285a80). pnpm build ✅ · pnpm lint ✅. Full-suite green is
demonstrated on the combined tree (this + #57): 220/220 suites, 1727/1727 tests.

🤖 Generated with Claude Code

@valthon
valthon force-pushed the fix/test-gate-port-lock-scoping branch from 6729c04 to 3477901 Compare August 20, 2026 15:18
@valthon
valthon force-pushed the fix/test-gate-port-lock-scoping branch from 3477901 to fe7ce4b Compare August 20, 2026 15:45
@valthon
valthon changed the base branch from master to fix/SOL-375-379-380-opp-collateral August 20, 2026 15:46
@valthon
valthon force-pushed the fix/test-gate-port-lock-scoping branch from fe7ce4b to 05f4c0f Compare August 20, 2026 16:46
@valthon
valthon changed the base branch from fix/SOL-375-379-380-opp-collateral to master August 20, 2026 16:46
Concurrent clusters on one host race the cross-process bind registry.
The advisory port lock now keys on the NORMALIZED registry path — two
spellings of one directory hash to one lock — and lives in the OS temp
dir rather than inside the registry: a sandboxing caller owns and
deletes that directory, while proper-lockfile keeps an mtime-refresh
timer alive for a held lock and its onCompromised hook THROWS when the
directory vanishes underneath it (`ENOENT ... wire-cluster-ports.lock.lock`).
Hashing the path keeps the scoping while putting the file somewhere no
test owns.

Real runs are unchanged: with the env var unset every process resolves
the same default registry, hence the same lock, and the host-global
cross-process guarantee holds exactly as before. A caller that sandboxes
the registry (every jest suite, via tests/jest.setup.ts) gets its OWN
lock, which is correct — disjoint registries cannot collide on a port,
so serializing them protects nothing.

The stale-lock steal window is raised 10s -> 30s so it sits ABOVE the
~15s loaded-host port-probing critical section; at 10s a live holder was
stolen mid-section, which is the failure mode that let two clusters draw
the same port and UDP-double-bind.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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