net/http: server.ListenAndServe should honour SetKeepAlivesEnabled

[aviddiviner]

Calling http.ListenAndServe will do a net.Listen(...) and then wrap that TCPListener in a tcpKeepAliveListener before calling Serve to handle any incoming connections.

This is clear from the docs on ListenAndServe, which states that:

Accepted connections are configured to enable TCP keep-alives.

But, http.Server also provides the handy SetKeepAlivesEnabled method. So, it would seem to me that calling SetKeepAlivesEnabled(false) and then ListenAndServe() would do the obvious thing and disable keep-alives on that server's connections.

... which it seems to do — sending Connection: close, and likely closing it off after writing the response — however...

As far as I can tell the tcpKeepAliveListener will still always call SetKeepAlive(true) and SetKeepAlivePeriod(3 * time.Minute) on each new connection, triggering a bunch of syscall.SetsockoptInt(fd.sysfd, syscall.SOL_SOCKET, syscall.SO_KEEPALIVE, boolint(keepalive)) magic each time.

Which can be avoided, especially if I'm disabling keep-alives before I even start serving my traffic.

So at the very least, this could be as simple as something like:

diff --git a/src/net/http/server.go b/src/net/http/server.go
index 89574a8..dd21955 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2216,7 +2216,10 @@ func (srv *Server) ListenAndServe() error {
 	if err != nil {
 		return err
 	}
-	return srv.Serve(tcpKeepAliveListener{ln.(*net.TCPListener)})
+	if srv.doKeepAlives() {
+		return srv.Serve(tcpKeepAliveListener{ln.(*net.TCPListener)})
+	}
+	return srv.Serve(ln)
 }
 
 var testHookServerServe func(*Server, net.Listener) // used if non-nil

And doing the same on ListenAndServeTLS.

I haven't looked how feasible it would be to have the tcpKeepAliveListener itself stop setting socket keep-alives once the server setting is changed, but I guess that would be even better?

Am I missing something? Let me know what you think.

What version of Go are you using (go version)?

go version go1.7.4 darwin/amd64

What operating system and processor architecture are you using (go env)?

GOHOSTARCH="amd64"
GOHOSTOS="darwin"

What did you do?

package main

import "net/http"

func main() {
	s := &http.Server{Addr: ":8080"}
	s.SetKeepAlivesEnabled(false)
	s.ListenAndServe()
}

[odeke-em]

/cc @bradfitz

[bradfitz]

You're conflating two different types of keep-alives. The docs distinguish between the two by saying "TCP keep-alives" vs "HTTP keep-alives".

The method you cite is about TCP keep-alives:

SetKeepAlivesEnabled controls whether HTTP keep-alives are enabled.

It has no control over TCP keep-alives as designed.

You can pass in your own TCP listener if you want to modify the default ListenAndServe behavior.

[aviddiviner]

Merry Chrismas! 🎄

@bradfitz: Thanks, I understand. Is there any benefit though to enabling TCP keep-alives when we know that no HTTP keep-alives will be performed? Surely that's unnecessary overhead on each connection that can be avoided?

[Shadas]

Thanks for your issue.
I'm using the default ListenAndServe function, and it will prints like "http: Accept error: accept tcp [::]:8077: accept4: too many open files; retrying in 1s". I checked the code after, I'm thinking if the error is because of the "tc.SetKeepAlivePeriod(3 * time.Minute)".