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: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/dial.go b/src/net/dial.go
index 4b430de4bde711a7fda85e30f554606c0ec6ac50..cb4ec216d53f3c3497f9a63f59ed4fdc19c450b7 100644 (file)
--- a/src/net/dial.go
+++ b/src/net/dial.go
@@ -75,8 +75,7 @@ func (d *Dialer) deadline(now time.Time) time.Time {
 
 // partialDeadline returns the deadline to use for a single address,
 // when multiple addresses are pending.
-func (d *Dialer) partialDeadline(now time.Time, addrsRemaining int) (time.Time, error) {
-       deadline := d.deadline(now)
+func partialDeadline(now, deadline time.Time, addrsRemaining int) (time.Time, error) {
        if deadline.IsZero() {
                return deadline, nil
        }
@@ -198,6 +197,7 @@ func DialTimeout(network, address string, timeout time.Duration) (Conn, error) {
 type dialContext struct {
        Dialer
        network, address string
+       finalDeadline    time.Time
 }
 
 // Dial connects to the address on the named network.
@@ -205,15 +205,17 @@ type dialContext struct {
 // See func Dial for a description of the network and address
 // parameters.
 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 {
                return nil, &OpError{Op: "dial", Net: network, Source: nil, Addr: nil, Err: err}
        }
 
        ctx := &dialContext{
-               Dialer:  *d,
-               network: network,
-               address: address,
+               Dialer:        *d,
+               network:       network,
+               address:       address,
+               finalDeadline: finalDeadline,
        }
 
        var primaries, fallbacks addrList
@@ -318,7 +320,7 @@ func dialSerial(ctx *dialContext, ras addrList, cancel <-chan struct{}) (Conn, e
                default:
                }
 
-               partialDeadline, err := ctx.partialDeadline(time.Now(), len(ras)-i)
+               partialDeadline, err := partialDeadline(time.Now(), ctx.finalDeadline, len(ras)-i)
                if err != nil {
                        // Ran out of time.
                        if firstErr == nil {

diff --git a/src/net/dial_test.go b/src/net/dial_test.go
index 9848f30e3c6912a9e13cf7651b5892af63fe499d..cfd7e092e46b30c4f8de345fd7e8d355c5b34029 100644 (file)
--- a/src/net/dial_test.go
+++ b/src/net/dial_test.go
@@ -228,10 +228,19 @@ func slowDialTCP(net string, laddr, raddr *TCPAddr, deadline time.Time) (*TCPCon
        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")
        if err != nil {
-               return 999 * time.Hour
+               return 999 * time.Hour, expected
        }
        addr := l.Addr().String()
        l.Close()
@@ -246,7 +255,7 @@ func dialClosedPort() time.Duration {
                }
                elapsed := time.Now().Sub(startTime)
                if i == 2 {
-                       return elapsed
+                       return elapsed, expected
                }
        }
 }
@@ -259,16 +268,7 @@ func TestDialParallel(t *testing.T) {
                t.Skip("both IPv4 and IPv6 are required")
        }
 
-       // Determine the time required to dial a closed port.
-       // 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
-       }
+       closedPortDelay, expectClosedPortDelay := dialClosedPort()
        if closedPortDelay > expectClosedPortDelay {
                t.Errorf("got %v; want <= %v", closedPortDelay, expectClosedPortDelay)
        }
@@ -551,8 +551,7 @@ func TestDialerPartialDeadline(t *testing.T) {
                {now.Add(1 * time.Millisecond), now, 1, noDeadline, errTimeout},
        }
        for i, tt := range testCases {
-               d := Dialer{Deadline: tt.deadline}
-               deadline, err := d.partialDeadline(tt.now, tt.addrs)
+               deadline, err := partialDeadline(tt.now, tt.deadline, tt.addrs)
                if err != tt.expectErr {
                        t.Errorf("#%d: got %v; want %v", i, err, tt.expectErr)
                }
@@ -605,6 +604,11 @@ func TestDialerDualStack(t *testing.T) {
                t.Skip("both IPv4 and IPv6 are required")
        }
 
+       closedPortDelay, expectClosedPortDelay := dialClosedPort()
+       if closedPortDelay > expectClosedPortDelay {
+               t.Errorf("got %v; want <= %v", closedPortDelay, expectClosedPortDelay)
+       }
+
        origTestHookLookupIP := testHookLookupIP
        defer func() { testHookLookupIP = origTestHookLookupIP }()
        testHookLookupIP = lookupLocalhost
@@ -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} {
                dss, err := newDualStackServer([]streamListener{
                        {network: "tcp4", address: "127.0.0.1"},
@@ -632,7 +636,7 @@ func TestDialerDualStack(t *testing.T) {
                        t.Fatal(err)
                }
 
-               d := &Dialer{DualStack: dualstack, Timeout: T}
+               d := &Dialer{DualStack: dualstack, Timeout: timeout}
                for range dss.lns {
                        c, err := d.Dial("tcp", JoinHostPort("localhost", dss.port))
                        if err != nil {
@@ -648,7 +652,7 @@ func TestDialerDualStack(t *testing.T) {
                        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) {