atunnel: close the relay's both ends before returning

[NekoPunch (orangeCatDeveloper)]

Fixes #1100

TestRelayIngressCancellationClosesBothSides fails on about 1% of -race runs, on branches unrelated to the relay.

Root cause

The test asserts a close that happens on a goroutine it never waits for.

relayIngressWithHalfClose closed both sides from a context.AfterFunc callback, which runs on its own goroutine. cancel() returning therefore says nothing about whether the streams are closed — and the test asserted right after it.

The client side is an unbuffered io.Pipe, where a Write cannot complete on its own; it needs someone on the other end. Two goroutines can be that someone:

Whichever gets there first	Outcome
the AfterFunc callback closes the pipe	Write fails → test passes
the relay's io.Copy, still parked in Read	it takes the bytes, Write returns nil → test fails

The scheduler picks the winner, so the test is a coin flip. -race widens the window by two orders of magnitude — 1 failure per 10000 plain, 89 with -race — and CI runs go test -race -v ./....

The actor.Read assertion just above races the same close, but carries a one-second deadline and so almost always wins. The Write has no such slack, which is why the failure is always line 123.

Fix

Production. Close both sides in the ctx.Done() branch the relay loop already selects on, rather than from a detached goroutine. This turns

both sides are eventually closed

into

both sides are closed before this returns

which is what a caller can build on, and it drops a goroutine plus its stop() bookkeeping.

Test. Wait for the relay to return, then assert. The SetReadDeadline error check goes with it: once the relay has closed the pipe, setting a deadline on it fails by design, and the deadline was only ever a guard against hanging if the connection had been left open.

Nothing changes about the bytes the relay moves — this is a shutdown-ordering fix, not a data-plane one.

Before / after

$ go test ./internal/atunnel/ -race -run TestRelayIngressCancellationClosesBothSides -count=10000
--- FAIL: TestRelayIngressCancellationClosesBothSides (0.00s)
    ingress_test.go:123: client stream remained open after relay cancellation
... 89 of 10000 iterations
FAIL	github.com/agent-substrate/substrate/internal/atunnel

$ go test ./internal/atunnel/ -race -run TestRelayIngressCancellationClosesBothSides -count=10000
ok  	github.com/agent-substrate/substrate/internal/atunnel	3.035s

Linux/x86_64, 4 cores, matching CI. On darwin/arm64 the same runs give 202 failures before and 0 after.

• Tests pass
• Appropriate changes to documentation are included in the PR

[Keith Mattix II (keithmattix)]

Thanks for the fit

[Aditya Shantanu (aditya-shantanu)]

Verified the analysis and the fix.

• The race is exactly as described: the context.AfterFunc closes ran on a detached goroutine, so cancel() returning guaranteed nothing about stream state, and the unbuffered io.Pipe write in the test raced the relay's io.Copy against the close. Moving the closes into the ctx.Done() branch of the relay loop makes "both sides closed before return" an actual postcondition, and the copy goroutines are still unblocked by those same closes (the done channel is buffered, so no leak).
• Behavior on the normal completion path is unchanged — the relay still doesn't close either side when both copies finish, same as before with defer stop().
• Ran go test -race ./internal/atunnel/ -run TestRelayIngressCancellationClosesBothSides -count=5000 locally (darwin/arm64): green. Also green with this branch merged into current main, -race -count=50 over the relay/serve tests.

LGTM.

[Aditya Shantanu (aditya-shantanu)]

Heads-up: adopted this fix (unchanged, with Co-authored-by credit) into the consolidated flake-fix PR #1160 to get CI green faster. If maintainers prefer landing this PR individually, #1160 can drop the commit or be closed — whatever lands first wins.