net: wait for cancelation goroutine before returning from connect.

This fixes a race which made it possible to cancel a connection after
returning from net.Dial.

Fixes #15035
Fixes #15078

Change-Id: Iec6215009538362f7ad9f408a33549f3e94d1606
Reviewed-on: https://go-review.googlesource.com/21497
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/dial_test.go b/src/net/dial_test.go
index 04e0fdae4498ec04d0dbc024bbe4cb8b0acfe674..2fc75c6356327b5181c31a6cf4ea9f521ed6c63c 100644 (file)
--- a/src/net/dial_test.go
+++ b/src/net/dial_test.go
@@ -5,6 +5,7 @@
 package net
 
 import (
+       "bufio"
        "internal/testenv"
        "io"
        "net/internal/socktest"
@@ -871,3 +872,84 @@ func TestDialCancel(t *testing.T) {
                }
        }
 }
+
+func TestCancelAfterDial(t *testing.T) {
+       if testing.Short() {
+               t.Skip("avoiding time.Sleep")
+       }
+
+       ln, err := newLocalListener("tcp")
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       var wg sync.WaitGroup
+       wg.Add(1)
+       defer func() {
+               ln.Close()
+               wg.Wait()
+       }()
+
+       // Echo back the first line of each incoming connection.
+       go func() {
+               for {
+                       c, err := ln.Accept()
+                       if err != nil {
+                               break
+                       }
+                       rb := bufio.NewReader(c)
+                       line, err := rb.ReadString('\n')
+                       if err != nil {
+                               t.Error(err)
+                               c.Close()
+                               continue
+                       }
+                       if _, err := c.Write([]byte(line)); err != nil {
+                               t.Error(err)
+                       }
+                       c.Close()
+               }
+               wg.Done()
+       }()
+
+       try := func() {
+               cancel := make(chan struct{})
+               d := &Dialer{Cancel: cancel}
+               c, err := d.Dial("tcp", ln.Addr().String())
+
+               // Immediately after dialing, request cancelation and sleep.
+               // Before Issue 15078 was fixed, this would cause subsequent operations
+               // to fail with an i/o timeout roughly 50% of the time.
+               close(cancel)
+               time.Sleep(10 * time.Millisecond)
+
+               if err != nil {
+                       t.Fatal(err)
+               }
+               defer c.Close()
+
+               // Send some data to confirm that the connection is still alive.
+               const message = "echo!\n"
+               if _, err := c.Write([]byte(message)); err != nil {
+                       t.Fatal(err)
+               }
+
+               // The server should echo the line, and close the connection.
+               rb := bufio.NewReader(c)
+               line, err := rb.ReadString('\n')
+               if err != nil {
+                       t.Fatal(err)
+               }
+               if line != message {
+                       t.Errorf("got %q; want %q", line, message)
+               }
+               if _, err := rb.ReadByte(); err != io.EOF {
+                       t.Errorf("got %v; want %v", err, io.EOF)
+               }
+       }
+
+       // This bug manifested about 50% of the time, so try it a few times.
+       for i := 0; i < 10; i++ {
+               try()
+       }
+}

diff --git a/src/net/fd_unix.go b/src/net/fd_unix.go
index c90e06847409d55f8d0f13c8ed323578b2ac1b01..d47b4bef996a66b8a9c62f43ab5ca6e328381c58 100644 (file)
--- a/src/net/fd_unix.go
+++ b/src/net/fd_unix.go
@@ -104,13 +104,17 @@ func (fd *netFD) connect(la, ra syscall.Sockaddr, deadline time.Time, cancel <-c
        }
        if cancel != nil {
                done := make(chan bool)
-               defer close(done)
+               defer func() {
+                       // This is unbuffered; wait for the goroutine before returning.
+                       done <- true
+               }()
                go func() {
                        select {
                        case <-cancel:
                                // Force the runtime's poller to immediately give
                                // up waiting for writability.
                                fd.setWriteDeadline(aLongTimeAgo)
+                               <-done
                        case <-done:
                        }
                }()

diff --git a/src/net/fd_windows.go b/src/net/fd_windows.go
index 7b8a91d482cc301e90d8d28c781bc52d114a9baf..100994525eb33cbd7d93ffe2724cb7d6496d36e7 100644 (file)
--- a/src/net/fd_windows.go
+++ b/src/net/fd_windows.go
@@ -352,14 +352,18 @@ func (fd *netFD) connect(la, ra syscall.Sockaddr, deadline time.Time, cancel <-c
        o := &fd.wop
        o.sa = ra
        if cancel != nil {
-               done := make(chan struct{})
-               defer close(done)
+               done := make(chan bool)
+               defer func() {
+                       // This is unbuffered; wait for the goroutine before returning.
+                       done <- true
+               }()
                go func() {
                        select {
                        case <-cancel:
                                // Force the runtime's poller to immediately give
                                // up waiting for writability.
                                fd.setWriteDeadline(aLongTimeAgo)
+                               <-done
                        case <-done:
                        }
                }()