Skip to content

Commit

Permalink
net: enable TCP keepalives by default
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
CAFxX authored and bradfitz committed Dec 3, 2018
1 parent 1adbb2b commit 5bd7e9c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 16 deletions.
14 changes: 10 additions & 4 deletions src/net/dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down
27 changes: 17 additions & 10 deletions src/net/dial_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/net/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

package net

import "context"
import (
"context"
"time"
)

var (
// if non-nil, overrides dialTCP.
Expand All @@ -19,5 +22,5 @@ var (
) ([]IPAddr, error) {
return fn(ctx, network, host)
}
testHookSetKeepAlive = func() {}
testHookSetKeepAlive = func(time.Duration) {}
)

0 comments on commit 5bd7e9c

Please sign in to comment.