net/http: possible issues with temporary errors from Accept

[alberts]

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]

Comment 1:

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

Status changed to Accepted.

[rsc]

Comment 2:

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

[rsc]

Comment 3:

Labels changed: added release-go1.3.

[rsc]

Comment 4:

Labels changed: removed go1.3.

[rsc]

Comment 5:

Labels changed: added repo-main.

[alberts]

Comment 6:

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

[bradfitz]

Comment 7:

https://golang.org/cl/97350043

Status changed to Started.

[gopherbot]

Comment 8:

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

[rsc]

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]

Comment 10:

Owner changed to @rsc.

[gopherbot]

Comment 11:

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

[rsc]

Comment 12:

This issue was closed by revision a07a57b.

Status changed to Fixed.

[gopherbot]

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

[gopherbot]

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