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

Generalizes the header write timeout to a broader "safety valve" write timeout. #11

Merged
merged 1 commit into from
Aug 20, 2015

Conversation

slackpad
Copy link
Contributor

This opens up the timeout to the user's payloads as well as just control headers. It's a little indirect because we don't have a timeout in the send() goroutine directly, but this timeout effectively covers that because the caller is waiting for that goroutine to put the write error onto the error feedback channel.

@armon
Copy link
Member

armon commented Aug 19, 2015

@slackpad Should we instead try to cast the connection to something with a SetWriteDeadline and use deadlines instead? That way we can save a ton of timer setup/teardown throughout.

@slackpad
Copy link
Contributor Author

@armon We could do that by moving the timeouts into session.go's send() loop, but then we don't cover the cases where the sendCh itself is filled up. I think we want timeout protection on those as well.

@armon
Copy link
Member

armon commented Aug 20, 2015

Ah makes sense! 👍

slackpad added a commit that referenced this pull request Aug 20, 2015
Generalizes the header write timeout to a broader "safety valve" write timeout.
@slackpad slackpad merged commit ae139c4 into master Aug 20, 2015
@slackpad slackpad deleted the f-safety-valve branch August 20, 2015 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants