Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

explicit read_timeout and write_timeout during hand/shake #3249

Merged
merged 2 commits into from
Feb 27, 2020

Conversation

antiochp
Copy link
Member

Resolves #3248.

We were blocking indefinitely on the initial hand/shake.
Once the peer is up and running for broadcast we will replace these timeout values with the existing settings for msg header and body reads and msg writes.

without these timeouts we see peer_connect threads stuck, blocked indefinitely waiting for read_exact on connections that may have closed suddenly.

@antiochp antiochp added the bug label Feb 26, 2020
@jaspervdm jaspervdm added this to the 3.1.0 milestone Feb 26, 2020
@jaspervdm
Copy link
Contributor

Given that the handshake reading is not in a loop like in the reader thread of poll(), it would mean that the connection is dropped if no message is received within HANDSHAKE_TIMEOUT. So in that case 2 seconds is maybe a bit too short.

@antiochp
Copy link
Member Author

antiochp commented Feb 26, 2020

Given that the handshake reading is not in a loop like in the reader thread of poll(), it would mean that the connection is dropped if no message is received within HANDSHAKE_TIMEOUT. So in that case 2 seconds is maybe a bit too short.

That's a good point - this would need to cover the period waiting for a shake after first sending a hand msg to a peer as we immediately try to read from the stream after sending.
I'll set these both to 10 seconds and see how it behaves.
Or maybe we need different values for read and write - let me take another look.

@antiochp antiochp merged commit f238058 into mimblewimble:master Feb 27, 2020
@antiochp antiochp deleted the handshake_timeout branch February 27, 2020 07:49
@antiochp antiochp mentioned this pull request Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no read/write timeout on initial hand/shake messages
2 participants