[TOB] Clean up the list of connecting peers in case of handshake timeout #2729
Conversation
This file contains 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
ljedrz
force-pushed
the
fix/handshake_timeout_cleanup
branch
from
5ba4d3c
to
0942507
Compare
|
Rebased and applied the same fix for the |
Signed-off-by: ljedrz <ljedrz@gmail.com>
…ke timeout Signed-off-by: ljedrz <ljedrz@gmail.com>
ljedrz
force-pushed
the
fix/handshake_timeout_cleanup
branch
from
0942507
to
39aa366
Compare
|
It probably works but isn't a great design. |
|
Alternatives are difficult, because there is no high-level callsite where an inbound connection timeout can be handled; having that would break the modularity of the network stack, and would be significantly more complex than this proposal. |
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.
Supersedes https://github.com/AleoHQ/snarkOS/pull/2690 as a much simpler and tighter solution.
I verified that this works by manually reducing the handshake timeout to 1ms and checking the number of connecting peers in 2 nodes after a timed out handshake between them (which is non-zero in
testnet3and zero with this PR). While adding a regular test case for this would be possible, it would be quite verbose (as one of the nodes would need to use a custom handshake impl), so I didn't include one.This change should obviate the current extra
Disconnectcheck, but I'm leaving it in for now just to be sure we're no longer hitting it.Finding: TOB-ALEO-22