Commit dcc905ec authored by Paul Marks's avatar Paul Marks Committed by Brad Fitzpatrick

net: compute the Dialer deadline exactly once.

When dialing with a relative Timeout instead of an absolute Deadline,
the deadline function only makes sense if called before doing any
time-consuming work.

This change calls deadline exactly once, storing the result until the
Dial operation completes.  The partialDeadline implementation is
reverted to the following patch set 3:
https://go-review.googlesource.com/#/c/8768/3..4/src/net/dial.go

Otherwise, when dialing a name with multiple IP addresses, or when DNS
is slow, the recomputed deadline causes the total Timeout to exceed that
requested by the user.

Fixes #11796

Change-Id: I5e1f0d545f9e86a4e0e2ac31a9bd108849cf0fdf
Reviewed-on: https://go-review.googlesource.com/12442
Run-TryBot: Paul Marks <pmarks@google.com>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
parent 92390e47
...@@ -75,8 +75,7 @@ func (d *Dialer) deadline(now time.Time) time.Time { ...@@ -75,8 +75,7 @@ func (d *Dialer) deadline(now time.Time) time.Time {
// partialDeadline returns the deadline to use for a single address, // partialDeadline returns the deadline to use for a single address,
// when multiple addresses are pending. // when multiple addresses are pending.
func (d *Dialer) partialDeadline(now time.Time, addrsRemaining int) (time.Time, error) { func partialDeadline(now, deadline time.Time, addrsRemaining int) (time.Time, error) {
deadline := d.deadline(now)
if deadline.IsZero() { if deadline.IsZero() {
return deadline, nil return deadline, nil
} }
...@@ -198,6 +197,7 @@ func DialTimeout(network, address string, timeout time.Duration) (Conn, error) { ...@@ -198,6 +197,7 @@ func DialTimeout(network, address string, timeout time.Duration) (Conn, error) {
type dialContext struct { type dialContext struct {
Dialer Dialer
network, address string network, address string
finalDeadline time.Time
} }
// Dial connects to the address on the named network. // Dial connects to the address on the named network.
...@@ -205,7 +205,8 @@ type dialContext struct { ...@@ -205,7 +205,8 @@ type dialContext struct {
// See func Dial for a description of the network and address // See func Dial for a description of the network and address
// parameters. // parameters.
func (d *Dialer) Dial(network, address string) (Conn, error) { func (d *Dialer) Dial(network, address string) (Conn, error) {
addrs, err := resolveAddrList("dial", network, address, d.deadline(time.Now())) finalDeadline := d.deadline(time.Now())
addrs, err := resolveAddrList("dial", network, address, finalDeadline)
if err != nil { if err != nil {
return nil, &OpError{Op: "dial", Net: network, Source: nil, Addr: nil, Err: err} return nil, &OpError{Op: "dial", Net: network, Source: nil, Addr: nil, Err: err}
} }
...@@ -214,6 +215,7 @@ func (d *Dialer) Dial(network, address string) (Conn, error) { ...@@ -214,6 +215,7 @@ func (d *Dialer) Dial(network, address string) (Conn, error) {
Dialer: *d, Dialer: *d,
network: network, network: network,
address: address, address: address,
finalDeadline: finalDeadline,
} }
var primaries, fallbacks addrList var primaries, fallbacks addrList
...@@ -318,7 +320,7 @@ func dialSerial(ctx *dialContext, ras addrList, cancel <-chan struct{}) (Conn, e ...@@ -318,7 +320,7 @@ func dialSerial(ctx *dialContext, ras addrList, cancel <-chan struct{}) (Conn, e
default: default:
} }
partialDeadline, err := ctx.partialDeadline(time.Now(), len(ras)-i) partialDeadline, err := partialDeadline(time.Now(), ctx.finalDeadline, len(ras)-i)
if err != nil { if err != nil {
// Ran out of time. // Ran out of time.
if firstErr == nil { if firstErr == nil {
......
...@@ -228,10 +228,19 @@ func slowDialTCP(net string, laddr, raddr *TCPAddr, deadline time.Time) (*TCPCon ...@@ -228,10 +228,19 @@ func slowDialTCP(net string, laddr, raddr *TCPAddr, deadline time.Time) (*TCPCon
return c, err return c, err
} }
func dialClosedPort() time.Duration { func dialClosedPort() (actual, expected time.Duration) {
// Estimate the expected time for this platform.
// On Windows, dialing a closed port takes roughly 1 second,
// but other platforms should be instantaneous.
if runtime.GOOS == "windows" {
expected = 1095 * time.Millisecond
} else {
expected = 95 * time.Millisecond
}
l, err := Listen("tcp", "127.0.0.1:0") l, err := Listen("tcp", "127.0.0.1:0")
if err != nil { if err != nil {
return 999 * time.Hour return 999 * time.Hour, expected
} }
addr := l.Addr().String() addr := l.Addr().String()
l.Close() l.Close()
...@@ -246,7 +255,7 @@ func dialClosedPort() time.Duration { ...@@ -246,7 +255,7 @@ func dialClosedPort() time.Duration {
} }
elapsed := time.Now().Sub(startTime) elapsed := time.Now().Sub(startTime)
if i == 2 { if i == 2 {
return elapsed return elapsed, expected
} }
} }
} }
...@@ -259,16 +268,7 @@ func TestDialParallel(t *testing.T) { ...@@ -259,16 +268,7 @@ func TestDialParallel(t *testing.T) {
t.Skip("both IPv4 and IPv6 are required") t.Skip("both IPv4 and IPv6 are required")
} }
// Determine the time required to dial a closed port. closedPortDelay, expectClosedPortDelay := dialClosedPort()
// On Windows, this takes roughly 1 second, but other platforms
// are expected to be instantaneous.
closedPortDelay := dialClosedPort()
var expectClosedPortDelay time.Duration
if runtime.GOOS == "windows" {
expectClosedPortDelay = 1095 * time.Millisecond
} else {
expectClosedPortDelay = 95 * time.Millisecond
}
if closedPortDelay > expectClosedPortDelay { if closedPortDelay > expectClosedPortDelay {
t.Errorf("got %v; want <= %v", closedPortDelay, expectClosedPortDelay) t.Errorf("got %v; want <= %v", closedPortDelay, expectClosedPortDelay)
} }
...@@ -551,8 +551,7 @@ func TestDialerPartialDeadline(t *testing.T) { ...@@ -551,8 +551,7 @@ func TestDialerPartialDeadline(t *testing.T) {
{now.Add(1 * time.Millisecond), now, 1, noDeadline, errTimeout}, {now.Add(1 * time.Millisecond), now, 1, noDeadline, errTimeout},
} }
for i, tt := range testCases { for i, tt := range testCases {
d := Dialer{Deadline: tt.deadline} deadline, err := partialDeadline(tt.now, tt.deadline, tt.addrs)
deadline, err := d.partialDeadline(tt.now, tt.addrs)
if err != tt.expectErr { if err != tt.expectErr {
t.Errorf("#%d: got %v; want %v", i, err, tt.expectErr) t.Errorf("#%d: got %v; want %v", i, err, tt.expectErr)
} }
...@@ -605,6 +604,11 @@ func TestDialerDualStack(t *testing.T) { ...@@ -605,6 +604,11 @@ func TestDialerDualStack(t *testing.T) {
t.Skip("both IPv4 and IPv6 are required") t.Skip("both IPv4 and IPv6 are required")
} }
closedPortDelay, expectClosedPortDelay := dialClosedPort()
if closedPortDelay > expectClosedPortDelay {
t.Errorf("got %v; want <= %v", closedPortDelay, expectClosedPortDelay)
}
origTestHookLookupIP := testHookLookupIP origTestHookLookupIP := testHookLookupIP
defer func() { testHookLookupIP = origTestHookLookupIP }() defer func() { testHookLookupIP = origTestHookLookupIP }()
testHookLookupIP = lookupLocalhost testHookLookupIP = lookupLocalhost
...@@ -618,7 +622,7 @@ func TestDialerDualStack(t *testing.T) { ...@@ -618,7 +622,7 @@ func TestDialerDualStack(t *testing.T) {
} }
} }
const T = 100 * time.Millisecond var timeout = 100*time.Millisecond + closedPortDelay
for _, dualstack := range []bool{false, true} { for _, dualstack := range []bool{false, true} {
dss, err := newDualStackServer([]streamListener{ dss, err := newDualStackServer([]streamListener{
{network: "tcp4", address: "127.0.0.1"}, {network: "tcp4", address: "127.0.0.1"},
...@@ -632,7 +636,7 @@ func TestDialerDualStack(t *testing.T) { ...@@ -632,7 +636,7 @@ func TestDialerDualStack(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
d := &Dialer{DualStack: dualstack, Timeout: T} d := &Dialer{DualStack: dualstack, Timeout: timeout}
for range dss.lns { for range dss.lns {
c, err := d.Dial("tcp", JoinHostPort("localhost", dss.port)) c, err := d.Dial("tcp", JoinHostPort("localhost", dss.port))
if err != nil { if err != nil {
...@@ -648,7 +652,7 @@ func TestDialerDualStack(t *testing.T) { ...@@ -648,7 +652,7 @@ func TestDialerDualStack(t *testing.T) {
c.Close() c.Close()
} }
} }
time.Sleep(2 * T) // wait for the dial racers to stop time.Sleep(timeout * 3 / 2) // wait for the dial racers to stop
} }
func TestDialerKeepAlive(t *testing.T) { func TestDialerKeepAlive(t *testing.T) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment