test(runtime): shut the peer down before dropping it in the broken-pipe test - #2881
Open
gertybotbot wants to merge 1 commit into
Open
test(runtime): shut the peer down before dropping it in the broken-pipe test#2881gertybotbot wants to merge 1 commit into
gertybotbot wants to merge 1 commit into
Conversation
…pe test `framed_send_to_broken_pipe_fails_closed_without_signal` established its "peer is gone" precondition with `drop(far)` alone. That releases the fd NUMBER back to the process while the rest of the suite is opening sockets and files on many threads. When another thread is handed the just-freed number before the send, the near end is connected to something live again: `framed_send` writes the whole payload and returns 4096 instead of -1. That is a harness race, not a regression in the per-socket SIGPIPE defenses the test exists to guard -- the reported `left: 4096` is a fully successful write, not an error code. Shut the peer down at the socket level before releasing its fd, so the precondition no longer depends on the fd slot staying free. Control (peer kept live to model fd reuse): drop-only sends 4096, reproducing the reported value exactly; shutdown-then-drop returns -1. So the change removes the race rather than merely passing. hew-runtime --lib green 8x; the broken-pipe test passed 8/8 (one unrelated scheduler_wasm sleep-dispatch flake appeared, filed separately). clippy --all-targets -D warnings RC=0, cargo fmt --all --check RC=0. Refs hew-lang#2877
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
1 participant
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.
Closes #2877.
What was wrong
framed_send_to_broken_pipe_fails_closed_without_signalestablished its "peer is gone" precondition withdrop(far)alone. That returns the fd number to the process while the rest of the suite opens sockets and files across many threads. If another thread receives the just-freed number before the send, the near end is connected to something live again —framed_sendwrites the whole payload and returns4096instead of-1.The reported value is the tell:
left: 4096is a fully successful write, not an error code. So this is a harness fd-reuse race, not a regression in theMSG_NOSIGNAL/SO_NOSIGPIPEdefenses the test exists to guard. Those defenses are fine; the precondition silently stops holding.Fix
Shut the peer down at the socket level before releasing its fd, so the connection is torn down rather than relying on the fd slot staying free. This is option 1 of the three the issue suggested — the smallest change, and it removes the race rather than narrowing it.
The test still deliberately omits the process-wide SIGPIPE ignore, so a genuine regression in either per-socket defense still kills the process loudly. That design is preserved.
Evidence
sendrcdrop-only (pre-fix)shutdown+drop(this PR)So the change discriminates rather than passing vacuously.
An earlier probe that
dup2d another socketpair onto the freed number returned-1in both arms and was discarded as invalid: it leavesneargenuinely disconnected, so it cannot reproduce the bug. Recording it because it is an easy control to write and to believe.Green bare (exit codes read directly, not through a pipe):
cargo test -p hew-runtime --lib— 8 full-suite runs, the broken-pipe test 8/8 ok (baseline in the issue: 5 ok / 1 FAILED over 6).cargo clippy -p hew-runtime --all-targets -- -D warnings— RC=0cargo fmt --all --check— RC=0Side finding, deliberately not folded in
One of the 8 runs failed on an unrelated test,
scheduler_wasm::tests::actor_is_parked_after_sleep_request_in_dispatch(scheduler_wasm.rs:8208,left: 1 / right: 0). Different test, different module, untouched by this change. Flagging rather than bundling it.Scope check: this is the only
UnixStream::pair()site intransport.rs, so no sibling test carries the same pattern.