From: Carlo Alberto Ferraris <cafxx@strayorange.com>
Date: Sun, 15 Apr 2018 02:34:19 +0000 (+0900)
Subject: net: enable TCP keepalives by default
X-Git-Tag: go1.12beta1~192
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=5bd7e9c54f946eec95d32762e7e9e1222504bfc1;p=gostls13.git

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 76dcdc164c..1dd8690739 100644
--- 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 983338885d..3a2c59a2d1 100644
--- 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 5a1156378b..ea71803e22 100644
--- 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) {}
 )