net/http: connection leak on panic

[anacrolix]

Connections are not closed if panic(nil) occurs during net.http.(*conn).serve().

http://code.google.com/p/go/source/browse/src/pkg/net/http/server.go#596

Specifically this occurs in user code during handler.ServeHTTP().

http://code.google.com/p/go/source/browse/src/pkg/net/http/server.go#681

[bradfitz]

Comment 1:

Owner changed to @bradfitz.

Status changed to Accepted.

[rsc]

Comment 2:

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

[rsc]

Comment 3:

Labels changed: added go1.1.

[shanemhansen]

Comment 4:

Reading the source code for server.go, it seems reasonable to c.Close() in the defer and
remove c.Close() from the bottom of the function.
I suppose you could also just call c.Close() in the deferred function and at the bottom
of serve. The implementations seem safe to close twice.
Does one of these solutions sounds reasonable? I can go ahead and code it up.

[anacrolix]

Comment 5:

I think it's worth fixing this ASAP. panic(nil) anywhere here (and particularly in
unpredictable user code) is silently discarded, and leaks connections. Of course, panics
being primarily used for exceptional cases means this doesn't happen often enough to be
noticeable for some time, which is even worse.
I'm not sure why c, and c.rwc are closed separately, and didn't bother to dig into it. I
think the easiest solution is to stuff:
defer func() {
    if !c.hijacked {
        c.close()
    }
}()
At line 617, and remove c.close() from line 704.

[rsc]

Comment 6:

Labels changed: added size-m.

[rsc]

Comment 7:

Labels changed: added suggested.

[bradfitz]

Comment 8:

https://golang.org/cl/6971049

Status changed to Started.

[bradfitz]

Comment 9:

This issue was closed by revision 91934ff.

Status changed to Fixed.