P2P connections minor issue during close - #19
Open
zenaror wants to merge 24 commits into
Open
Conversation
Forgot va_end()...
This reverts commit 093209a.
Feature/custom mail port
Two commits already made recv_size == -2 (graceful remote close) silent for P2P calls, since a real Mobile Adapter over a phone line has no way to instantly know the line dropped -- the game is meant to notice via its own timeout instead (see "update P2P synchronisation"). The other two error exits in command_data() weren't consistent with that: a failed send() (rc < 0) and any other recv() error (recv_size < 0, e.g. ECONNRESET from a P2P peer that closed abruptly) both still raised an immediate Error Status regardless of P2P/internet mode. That's exactly the same kind of information a phone-line call couldn't have given the game either, and it's what was letting one side of a two-machine P2P test find out about the other side hanging up immediately while the other side never did (depends only on whether the OS happened to send a FIN or an RST at close time, not on anything meaningful). Both paths now stay silent for P2P the same way, consistent with the existing -2 handling. Also: command_wait_call_begin() didn't defensively close a leftover p2p_conn the way command_tel_begin() already does in the opposite direction (that function's own comment says as much: "Close any connection created by command_wait_call"). mobile_cb_sock_open() requires the connection slot to already be closed, so mirrored the same guard here for symmetry.
P2P: don't report low-level socket errors to the game either
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.
P2P: don't report low-level socket errors to the game either
Two previous commits already made
recv_size == -2(graceful remote close) silent for P2P calls.This matches the behavior of a real Mobile Adapter over a phone line: there is no way to instantly know that the line dropped, so the game is expected to detect the failure through its own timeout instead
However, the other two error exits in
command_data()were not consistent with that behavior:send()(rc < 0)recv()error (recv_size < 0), such asECONNRESETfrom a P2P peer that closed abruptlyBoth cases still raised an immediate Error Status, regardless of whether the connection was using P2P or internet mode.
This exposes exactly the same kind of low-level information that a phone-line call could not have provided to the game. It was also causing asymmetric behavior in two-machine P2P tests: one side could detect the other side hanging up immediately, while the other side would not.
That behavior depended only on whether the OS happened to send a
FINor anRSTwhen the socket was closed, rather than on anything meaningful at the protocol level.Both error paths now remain silent in P2P mode, consistent with the existing
recv_size == -2handling.Additionally,
command_wait_call_begin()did not defensively close a leftoverp2p_conn, whilecommand_tel_begin()already does so in the opposite direction. The existing comment in that function explicitly mentions this behavior:Since
mobile_cb_sock_open()requires the connection slot to already be closed, the same guard is now mirrored incommand_wait_call_begin()for symmetry.This PR also includes the mail port redirection #14