Skip to content

Give the no-PID keyed test a socket that refuses whoever holds it - #212

Merged
myobie merged 1 commit into
mainfrom
agent/keyed-no-pid-test-stale-socket
Nov 4, 2026
Merged

myobie merged 1 commit into
mainfrom
agent/keyed-no-pid-test-stale-socket

Conversation

@myobie

@myobie myobie commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

This fixes a test-only race behind one failure of registry::keyed::tests::a_record_naming_no_pid_is_probed_but_never_turned_into_a_loss. It failed in #210's Nix job and passed on rerun. The run got Found(Running, Unknown) where Indeterminate(SocketProbe) was expected (keyed.rs:1375).

Cause. The test drops its UnixListener and expects the next probe to be refused. A dropped listener can still accept connections:

  • pty-core's unit tests run on parallel threads, and some of them spawn processes.
  • A child that another thread is spawning holds a copy of every descriptor, from its fork until its exec closes them.
  • A probe that connects in that window is accepted, and the observer correctly reads the session as running.

registry_support::stale_socket documents the same race and already avoids it for the integration tests. It failed 44 runs in 1000 on a Mac on 2026-09-28.

Fix. The test now replaces the dropped listener with a socket made the same way as stale_socket: bound and never listened on. Such a socket refuses every connect, whichever process holds it. The helper also checks that it refuses. The observer is unchanged.

Measured on Linux, at main d997dd3 (its pty-core is identical to this branch's base):

  • Deterministic control. Before the drop, I made another thread spawn true with a pre_exec that signals and then sleeps 300 ms, so its child holds the copies while the test probes. With that injection, main's test failed 5/5 with the CI symptom: Found(... status: Running, identity: Unknown) at the refusal assertion. This branch's test passed 5/5 under the same injection.

  • Natural rate. Main failed 0/2000 with the test alone, and 0/200 with the whole pty-core lib binary unloaded. Pinned to one core with two spinners, main and this branch ran side by side: the keyed test failed 0/60 on each.

    So on this host the window is rare. CI's build sandbox can be much slower to exec.

  • This branch: pty-core lib, 205 passed.

The one-core runs also caught an unrelated timing flake: operation::record::tests::an_observed_exit_is_gone_once_the_zombie_shows_within_the_cap failed 1/60 on main. I'm reporting it separately; it isn't touched here.

a_record_naming_no_pid_is_probed_but_never_turned_into_a_loss dropped
its UnixListener and expected the next probe to be refused. A dropped
listener can still accept: pty-core's unit tests run on parallel
threads, some spawn processes, and a child that another thread is
spawning holds a copy of every descriptor from its fork until its exec
closes them. A probe in that window connects, and the observer correctly
reads the session as running. That is the failure seen once in CI:
Found(Running, Unknown) where Indeterminate(SocketProbe) was expected.

registry_support::stale_socket already gives the integration tests a
socket that was bound and never listened on for this reason. Replace
the dropped listener with one made the same way. The observer is
unchanged.
@myobie

myobie commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Independent read GO at exact head 636bdb8ebddd0a64b07d232838bd4a2358264510. All 9 checks are SUCCESS on this head and the PR is CLEAN.

Read:

  • Test-only: one file (crates/pty-core/src/registry/keyed.rs, +38), no product code, no wire-format change, no tier-1 file (events.jsonl, <id>.json, docs/disk-layout.md).
  • The flaky test is a_record_naming_no_pid_is_probed_but_never_turned_into_a_loss. It dropped a UnixListener and expected the socket to refuse connections. A child that another test thread is spawning can hold a copy of the listener from its fork until its exec, so the dropped socket can still accept, and the observer then correctly reports the session as running. That is a race in the test, not in the product.
  • The fix removes the socket file and binds a socket that is never listened on, built the same way as the existing integration-test helper, and asserts the connect is refused before the observer runs. The test's intent and assertions are unchanged.
  • Per the author, a held listener copy injected into the real test made the old version fail 5 of 5 times and the fixed one pass 5 of 5; the race did not show at its natural rate in 2000 runs, which fits CI's slower exec.

Risks (low): the helper adds an unsafe block of plain socket calls to test code (the same technique as the integration helper), which CI compiled and ran.

@myobie
myobie merged commit 9e94bcb into main Nov 4, 2026
9 checks passed
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