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 golang#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 golang#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 golang#23459

Change-Id: I348c03be97588d5001e6de0f377e7a93b51957fd
  • Loading branch information
CAFxX committed Apr 15, 2018
1 parent b61b1d2 commit eee43bd
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 @@ -57,8 +57,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 @@ -400,10 +402,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 @@ -725,22 +725,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 @@ -18,5 +21,5 @@ var (
) ([]IPAddr, error) {
return fn(ctx, host)
}
testHookSetKeepAlive = func() {}
testHookSetKeepAlive = func(time.Duration) {}
)

0 comments on commit eee43bd

Please sign in to comment.