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

Change idle implementation to be graceful #41

Merged
merged 1 commit into from
Feb 2, 2015

Conversation

ncdc
Copy link
Contributor

@ncdc ncdc commented Feb 2, 2015

Replace the use of net.Conn's SetDeadline() in the idling implementation
with a custom monitor that gracefully resets streams and closes the
connection. Abrupt termination of the connection using SetDeadline()
results in things such as proxy servers being slow to detect idled
connections, meaning clients connected to the proxy aren't immediately
notified of the connection closure.

Also incorporates some of the code from
8239520 to fix the issue of being
unable to Reset() a stream if it has data waiting to be read.

Signed-off-by: Andy Goldstein agoldste@redhat.com

@dmcgowan
Copy link
Member

dmcgowan commented Feb 2, 2015

LGTM after comment for unprotected access to i.expired

Replace the use of net.Conn's SetDeadline() in the idling implementation
with a custom monitor that gracefully resets streams and closes the
connection. Abrupt termination of the connection using SetDeadline()
results in things such as proxy servers being slow to detect idled
connections, meaning clients connected to the proxy aren't immediately
notified of the connection closure.

Also incorporates some of the code from
8239520 to fix the issue of being
unable to Reset() a stream if it has data waiting to be read.

Signed-off-by: Andy Goldstein <agoldste@redhat.com>
@ncdc
Copy link
Contributor Author

ncdc commented Feb 2, 2015

This should fix the same issue described in #34

dmcgowan added a commit that referenced this pull request Feb 2, 2015
Change idle implementation to be graceful
@dmcgowan dmcgowan merged commit 13f2d13 into moby:master Feb 2, 2015
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