net: enable TCP keepalives by default

This is just the first step in attempting to make all network connection have
timeouts as a "safe default". TCP keepalives only protect against certain
classes of network and host issues (e.g. server/OS crash), but do nothing
against application-level issues (e.g. an application that accepts connections
but then fails to serve requests).

The actual keep-alive duration (15s) is chosen to cause broken connections
to be closed after 2~3 minutes (depending on the OS, see #23549 for details).
We don't make the actual default value part of the public API for a number of
reasons:
- because it's not very useful by itself: as discussed in #23549 the actual
  "timeout" after which the connection is torn down is duration*(KEEPCNT+1),
  and we use the OS-wide value for KEEPCNT because there's currently no way
  to set it from Go.
- because it may change in the future: if users need to rely on a specific
  value they should explicitly set this value instead of relying on the default.

Fixes #23459

Change-Id: I348c03be97588d5001e6de0f377e7a93b51957fd
Reviewed-on: https://go-review.googlesource.com/c/107196
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/dial.go b/src/net/dial.go
index 76dcdc164cabd0f98813c386b1a71a0f704f6ba7..1dd8690739ef64a79c31d5bf58557c1e3c764800 100644 (file)
--- a/src/net/dial.go
+++ b/src/net/dial.go
@@ -65,8 +65,10 @@ type Dialer struct {
 
        // KeepAlive specifies the keep-alive period for an active
        // network connection.
-       // If zero, keep-alives are not enabled. Network protocols
+       // If zero, keep-alives are enabled if supported by the protocol
+       // and operating system. Network protocols or operating systems
        // that do not support keep-alives ignore this field.
+       // If negative, keep-alives are disabled.
        KeepAlive time.Duration
 
        // Resolver optionally specifies an alternate resolver to use.
@@ -418,10 +420,14 @@ func (d *Dialer) DialContext(ctx context.Context, network, address string) (Conn
                return nil, err
        }
 
-       if tc, ok := c.(*TCPConn); ok && d.KeepAlive > 0 {
+       if tc, ok := c.(*TCPConn); ok && d.KeepAlive >= 0 {
                setKeepAlive(tc.fd, true)
-               setKeepAlivePeriod(tc.fd, d.KeepAlive)
-               testHookSetKeepAlive()
+               ka := d.KeepAlive
+               if d.KeepAlive == 0 {
+                       ka = 15 * time.Second
+               }
+               setKeepAlivePeriod(tc.fd, ka)
+               testHookSetKeepAlive(ka)
        }
        return c, nil
 }

diff --git a/src/net/dial_test.go b/src/net/dial_test.go
index 983338885dac52df8c0fbc93f804e67bf6e57f3e..3a2c59a2d1d6be90dba1db1eaa15f7080860097e 100644 (file)
--- a/src/net/dial_test.go
+++ b/src/net/dial_test.go
@@ -729,22 +729,29 @@ func TestDialerKeepAlive(t *testing.T) {
        if err := ls.buildup(handler); err != nil {
                t.Fatal(err)
        }
-       defer func() { testHookSetKeepAlive = func() {} }()
+       defer func() { testHookSetKeepAlive = func(time.Duration) {} }()
 
-       for _, keepAlive := range []bool{false, true} {
-               got := false
-               testHookSetKeepAlive = func() { got = true }
-               var d Dialer
-               if keepAlive {
-                       d.KeepAlive = 30 * time.Second
-               }
+       tests := []struct {
+               ka       time.Duration
+               expected time.Duration
+       }{
+               {-1, -1},
+               {0, 15 * time.Second},
+               {5 * time.Second, 5 * time.Second},
+               {30 * time.Second, 30 * time.Second},
+       }
+
+       for _, test := range tests {
+               var got time.Duration = -1
+               testHookSetKeepAlive = func(d time.Duration) { got = d }
+               d := Dialer{KeepAlive: test.ka}
                c, err := d.Dial("tcp", ls.Listener.Addr().String())
                if err != nil {
                        t.Fatal(err)
                }
                c.Close()
-               if got != keepAlive {
-                       t.Errorf("Dialer.KeepAlive = %v: SetKeepAlive called = %v, want %v", d.KeepAlive, got, !got)
+               if got != test.expected {
+                       t.Errorf("Dialer.KeepAlive = %v: SetKeepAlive set to %v, want %v", d.KeepAlive, got, test.expected)
                }
        }
 }

diff --git a/src/net/hook.go b/src/net/hook.go
index 5a1156378b569b98e571e819efb1d1c7accc7e00..ea71803e22a70397c43e91af037c31dcaed7a081 100644 (file)
--- a/src/net/hook.go
+++ b/src/net/hook.go
@@ -4,7 +4,10 @@
 
 package net
 
-import "context"
+import (
+       "context"
+       "time"
+)
 
 var (
        // if non-nil, overrides dialTCP.
@@ -19,5 +22,5 @@ var (
        ) ([]IPAddr, error) {
                return fn(ctx, network, host)
        }
-       testHookSetKeepAlive = func() {}
+       testHookSetKeepAlive = func(time.Duration) {}
 )