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: possible issues with temporary errors from Accept #6163

Closed
alberts opened this issue Aug 16, 2013 · 14 comments
Closed

net/http: possible issues with temporary errors from Accept #6163

alberts opened this issue Aug 16, 2013 · 14 comments
Milestone

Comments

@alberts
Copy link
Contributor

alberts commented Aug 16, 2013

What steps will reproduce the problem?

net/http's accept loop does this check on errors from Accept:

http://golang.org/src/pkg/net/http/server.go#L1544

if ne, ok := e.(net.Error); ok && ne.Temporary() {

which ends up calling into Temporary here:

http://golang.org/src/pkg/syscall/syscall_unix.go#L108

return e == EINTR || e == EMFILE || e.Timeout()

Timeout does:

return e == EAGAIN || e == EWOULDBLOCK || e == ETIMEDOUT

With other Go services, under certain rapid connection setup/teardown conditions, I've
seen Accept return ECONNRESET (connection reset by peer) on kernel 3.3.8. According to
the man page, it could also return ECONNABORTED.

I think at least ECONNRESET and maybe also ECONNABORTED should be treated as Temporary
by net/http's accept loop.

If Temporary() can't be changed, it would be nice to add some code somewhere (maybe in
net?) to help other people to write accept loops correctly.

One other minor note: if a process runs out of file descriptors for another reason than
accepting too many connections, the programmer might want EMFILE to be a fatal error,
especially if you have a Listener that limits the number of open HTTP connections. I've
seen this in the real world with a net/http server on top of a misbehaving LevelDB using
the levigo wrapper with the C implementation and large databases. EMFILE might also fall
out in other places in the program where it can be handled as a fatal error, but it
might not. I guess fixing this might be too complicated for Go 1.
@robpike
Copy link
Contributor

robpike commented Aug 16, 2013

Comment 1:

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Sep 10, 2013

Comment 2:

Labels changed: added go1.3, removed go1.2maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 3:

Labels changed: added release-go1.3.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 4:

Labels changed: removed go1.3.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 5:

Labels changed: added repo-main.

@alberts
Copy link
Contributor Author

alberts commented Jan 3, 2014

Comment 6:

See also issue #6987, which is this issue on Windows.

@bradfitz
Copy link
Contributor

Comment 7:

https://golang.org/cl/97350043

Status changed to Started.

@gopherbot
Copy link

Comment 8:

CL https://golang.org/cl/97350043 mentions this issue.

@rsc
Copy link
Contributor

rsc commented May 13, 2014

Comment 9:

Per discussion on CL, let's leave this for 1.4 but we do know how we're going to fix it.

Labels changed: added release-go1.4, removed release-go1.3.

@rsc
Copy link
Contributor

rsc commented Sep 16, 2014

Comment 10:

Owner changed to @rsc.

@gopherbot
Copy link

Comment 11:

CL https://golang.org/cl/141600043 mentions this issue.

@rsc
Copy link
Contributor

rsc commented Sep 19, 2014

Comment 12:

This issue was closed by revision a07a57b.

Status changed to Fixed.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@gopherbot
Copy link

CL https://golang.org/cl/9946 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/39490 mentions this issue.

wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
Fixes golang#6163.

LGTM=adg
R=golang-codereviews, adg, dvyukov
CC=golang-codereviews
https://golang.org/cl/141600043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
Fixes golang#6163.

LGTM=adg
R=golang-codereviews, adg, dvyukov
CC=golang-codereviews
https://golang.org/cl/141600043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
Fixes golang#6163.

LGTM=adg
R=golang-codereviews, adg, dvyukov
CC=golang-codereviews
https://golang.org/cl/141600043
@rsc rsc removed their assignment Jun 22, 2022
This issue was closed.
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