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

net/http/httputil: ReverseProxy doesn't forward Close notifications #8406

Closed
gopherbot opened this issue Jul 22, 2014 · 5 comments
Closed

net/http/httputil: ReverseProxy doesn't forward Close notifications #8406

gopherbot opened this issue Jul 22, 2014 · 5 comments
Milestone

Comments

@gopherbot
Copy link

by peter@scraperwiki.com:

This is a follow-up to
https://groups.google.com/d/topic/golang-dev/-pPRjs46QIk/discussion.

With the following code:

http://play.golang.org/p/Qrqg2XmuRz

Consider a long running http handler (L19) which hypothetically uses (e.g, CPU)
resources until the connection is closed. The handler is aware of connection closure
through: w.(http.CloseNotifier).CloseNotify().

If we proxy to that handler via a http.ReverseProxy, closes don't make it to the target
server (and it keeps consuming resources indefinitely).

One can see the correct behaviour when querying the original internal handler directly
and if line 72 (useExternal=true) is uncommented, the demo deadlocks.

The deadlock is because the internal server is waiting forever, since it never received
a close notification.

One can observe the same behaviour with the same demo application by using cURL from the
console and aborting cURL. If aborting against the internal server, then "[server]
Close Notified!" is seen. If aborting against the external, it is never seen.
@gopherbot
Copy link
Author

Comment 1 by peter@scraperwiki.com:

FWIW, line 29 of the linked example code is incorrectly a 'switch' when I meant
'select', but it doesn't affect the outcome.

@nigeltao
Copy link
Contributor

nigeltao commented Aug 6, 2014

Comment 2:

Owner changed to @bradfitz.

@griesemer
Copy link
Contributor

Comment 3:

Labels changed: added repo-main.

@bradfitz bradfitz added this to the Go1.5 milestone Dec 18, 2014
@bradfitz bradfitz removed the new label Dec 18, 2014
@pwaller
Copy link
Contributor

pwaller commented Dec 30, 2014

Thanks to @bradfitz for pinging me, I've made the implementation over here, except I can't use the contribution system right now. I hope to have another go in a few days.

pwaller added a commit to pwaller/go that referenced this issue Dec 30, 2014
@bradfitz
Copy link
Contributor

I'll wait on you to send a review to Gerrit before leaving comments.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants