Repository navigation
Relay: a stream leg accepted as the connect wait ends is used, not left open - #166
Merged
Merged
Conversation
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.
Why
A relay stream has two legs. The connecting side waits up to 10 seconds for the target to accept. The accepting side takes the stream out of the server's table, answers its player "ready", and then hands over its connection. If the connecting side's wait ended between those steps, the accepting player had been told the stream was ready, the connecting player was told it was not found, and the accepted connection stayed in the stream's channel with nothing to close it.
Change
server/relay.go: the wait moves intoawaitAccept(st, d). When the wait ends (timeout or server stop) it checks, under the server lock, whether the stream is still in the table. If it is, it is removed and there is no leg. If it is not, the accepting side has claimed it, andawaitAcceptwaits for its leg and returns it, so the stream is used as normal. The accepting side now always sends a value on the channel: its connection, or nil if the "ready" write failed. A stop while waiting still ends the connection silently, as before.server/relay_accept_test.go: three tests ofawaitAccept(a leg claimed as the wait ends is returned; a claimed stream whose accepting side failed gives nil; an unclaimed stream times out and is removed from the table).Tests
go vetandgo test -race ./serverpass.awaitAcceptreturn nil for a claimed stream failsTestAwaitAcceptUsesLegClaimedAtTimeout.python3 tools/features.py,python3 tools/site.py,python3 tools/repo_check.pyandscripts/check-deploy.shpass.Evidence
Before the change, a throwaway test (not in this PR) with the wait shortened to 4 ms and accept times spread around it: in 12,000 attempts, 3 had the accepting side told ready and the connecting side not, and the process's open descriptor count rose by 3. At the real 10 second wait the window is the same few microseconds, so it is rare. The new tests do not depend on timing.
Not in this change
The wait length is unchanged. No other relay behaviour changes.
Overlap
None open on
server/relay.go. Touches CHANGELOG.md,site/index.htmlandsite/data.json, which other open PRs also change; I will update this branch if one merges first.Developed with AI agent assistance.