Repository navigation
test(runtime-core): prove relay refusals without socket timeouts and reap failed relay starts - #508
Merged
Merged
Conversation
…reap failed relay starts HACK-1342 items 2 and 4 (item 1 is already fixed on next by #399; item 3 is left to #419). Item 2: the relay refuses an excess connection (and one whose target is down) by accepting and closing it, which can happen before the client's next syscall. Once the peer has closed, macOS rejects SO_RCVTIMEO and SO_SNDTIMEO with EINVAL, so `Relay::connect` panicked at the `set_read_timeout`/`set_write_timeout` unwraps. The relay is correct. The two refusal tests now use `Relay::connect_refused`, which sets no socket timeouts: it waits for readability with a bounded poll and then still requires EOF or a reset. A live peer or a peer that sends data still fails the check. Item 4: `Relay::start_options` now hands the spawned child to the existing `PublisherStartup` guard until readiness is proven, so a failed or panicking readiness check kills and reaps `stream-relay-host` (and removes a fresh root) instead of orphaning it. Refs HACK-1342 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ID (HACK-1342) Relay test roots were named /tmp/<prefix>-<pid>-<n> and created with create_dir().unwrap(), so a directory left by an earlier run whose PID is now reused failed a fresh test with EEXIST. fresh_root() skips an existing name, and every root site uses it, including the fixed-name ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix unit G112 for HACK-1342, items 1, 2 and 4. Test-harness only: one file changed (
packages/runtime-core/tests/stream_relay_contract.rs). The relay (guest/stream-relay.c) is not changed, because the evidence shows it behaves correctly.Item 3 (the 2 s deadlines in
provider/graph/native/runtime/tests.rs) is left to #419 (HACK-1367). This PR does not touchruntime/tests.rs,exec_tests.rs,run_tests.rsortest_budget.rs.Relay::connectEINVAL (CI run 40119166731,stream_relay_contract.rs:108:14)close()(stream-relay.c:489). For an unavailable target it callsrelease()afterconnect_targetfails. Both can happen before the client's next syscall. Once the peer has closed a stream socket, macOS (XNUsosetopt) rejectsSO_RCVTIMEO/SO_SNDTIMEOwith EINVAL, because both directions are shut.Relay::connectunwrapsset_read_timeout(line 105) andset_write_timeout(line 108), so it panics when the refusal arrives first. The hypothesis is confirmed. The relay is correct.Relay::connect_refused(). It sets no socket timeouts. It waits up toREFUSAL_DEADLINE(10 s, as before) withpolland then still requires EOF or a reset (assert_refused). A peer that stays open fails the check (timeout), and so does a peer that sends data.Relay::connectstill unwraps the timeout calls for streams the relay must keep, so EINVAL there is still a real failure.stream-relay-hoston a readiness panicstart_optionsspawned the child and ran its readiness assertions before anyRelayexisted, so a panic dropped theChildwithout killing it.await_bare_ready. Until readiness passes, the existingPublisherStartupguard owns the child. ItsDropkills and reaps the child, and removes a fresh scratch root (never a root the caller passed in).provider/publisher.rsprobe-port racenextby #399 (674a0a4): the fixture now runs--publish 0and readsready <port>. No probe listener is left.Evidence
Root cause (macOS 26.5.1, Darwin 25.5.0)
A standalone Rust repro (not committed):
set_read_timeout=Ok(())set_read_timeoutandset_write_timeout=Os { code: 22, InvalidInput };read=Ok(0)set_write_timeoutcalls return EINVALset_read_timeout=Ok(()). The error needs both directions to be shut, which matches XNU's check.Parallel load on this macOS host (
cargo +1.97.1, Zig 0.15.2 via mise, 8 concurrent processes ×--test-threads 16, host load average ~105–116 on 18 cores)stream_relay_contractbinaryconnection_limit,occupied_socket)connection_limit_refuses_excess...withOs { code: 22 }at theset_read_timeoutunwrap (:105:14)provider::publisherlib tests (--features native-stream-relay)The before failures appeared at line 105 locally and at line 108 on CI. Both are the same race; it depends on which
setsockoptthe relay'sclose()lands before.New tests, each proven by mutation
refusal_check_accepts_a_peer_that_closed_before_the_check: drops the peer before the check. Mutation: restore the old pattern (set timeouts, then read) inassert_refused_within. Result: it fails deterministically with EINVAL.refusal_check_rejects_a_live_peerandrefusal_check_rejects_a_peer_that_sends_data(should_panic). Mutation: make the check return early (accept anything). Result: both fail. The new check does not weaken the refusal proof.failed_relay_readiness_kills_and_reaps_the_child: a live child prints the wrong readiness line. The test then requireswaitpid(pid, WNOHANG)= ECHILD (already killed and reaped) and the fresh root removed. Mutation: take the child out of the guard before the checks (the old ownership). Result: it fails withE_RELAY_STARTUP_ORPHAN, and the test kills the child itself.Gates
cargo +1.97.1 fmt --check; clippy--all-targets -D warningswith default features (check:local) and with--all-features: cleantest:local(cargo +1.97.1 test --jobs 2): all pass (lib 1065 passed, 101 ignored)stream_relay_contractwithnative-stream-relay: 29/29bun run check(includesprivacy:check) andbun run --cwd packages/cli typecheck: 0 errorsorigin/next..HEAD(gitleaks diron agit log -pexport): no leakstestshards 1-4 andCandidate core (macos-latest)(optional feature contracts). This is the full-suite evidence.Gaps
sosetoptcheck. Linux does not show it, and the new helper does not depend on it either way.Round 2 (5752a32): stale scratch roots
The verifier hit one unrelated failure on
nextunder 32-process load:fs::create_dirat line 52 returned EEXIST. Roots are named/tmp/<prefix>-<pid>-<n>, and a directory left by an earlier run whose PID is now reused collides. The newfresh_root(prefix)skips an existing name and panics on any other error. Every root site uses it, including the fixed-namehkr-netns,hkr-reservation,hkp-ackandhkp-privateroots.fresh_root_skips_a_stale_directory_with_the_next_name. A mutation that stops skipping makes it fail.stream_relay_contract: 30/30 pass. 32 full-binary runs (4 rounds of 8 processes,--test-threads 16, load 92-154) all pass. fmt andclippy --all-targets --all-features -D warningsare clean. gitleaks found no leaks.net.inet.tcp.blackhole0) and is not refused, so that would turn the refusal into a timeout. The theoretical race is tracked separately.Refs HACK-1342
🤖 Generated with Claude Code