crypto/tls: retry DialWithTimeout until the listener accepts a connec…

…tion

The point of DialWithTimeout seems to be to test what happens when the
connection times out during handshake. However, the test wasn't
actually verifying that the connection made it into the handshake at
all. That would not only fail to test the intended behavior, but also
leak the Accept goroutine until arbitrarily later, at which point it
may call t.Error after the test t is already done.

Instead, we now:

- retry the test with a longer timeout if we didn't accept a
  connection, and

- wait for the Accept goroutine to actually complete when the test
  finishes.

Fixes golang#59646.

Change-Id: Ie56ce3297e2c183c02e67b8f6b26a71e50964558
Reviewed-on: https://go-review.googlesource.com/c/go/+/485115
Run-TryBot: Bryan Mills <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Commit-Queue: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

src/crypto/tls/tls_test.go

@@ -170,35 +170,59 @@ func TestDialTimeout(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping in short mode")
 	}
-	listener := newLocalListener(t)
 
-	addr := listener.Addr().String()
-	defer listener.Close()
-
-	complete := make(chan bool)
-	defer close(complete)
+	timeout := 100 * time.Microsecond
+	for !t.Failed() {
+		acceptc := make(chan net.Conn)
+		listener := newLocalListener(t)
+		go func() {
+			for {
+				conn, err := listener.Accept()
+				if err != nil {
+					close(acceptc)
+					return
+				}
+				acceptc <- conn
+			}
+		}()
 
-	go func() {
-		conn, err := listener.Accept()
-		if err != nil {
-			t.Error(err)
-			return
+		addr := listener.Addr().String()
+		dialer := &net.Dialer{
+			Timeout: timeout,
+		}
+		if conn, err := DialWithDialer(dialer, "tcp", addr, nil); err == nil {
+			conn.Close()
+			t.Errorf("DialWithTimeout unexpectedly completed successfully")
+		} else if !isTimeoutError(err) {
+			t.Errorf("resulting error not a timeout: %v\nType %T: %#v", err, err, err)
 		}
-		<-complete
-		conn.Close()
-	}()
 
-	dialer := &net.Dialer{
-		Timeout: 10 * time.Millisecond,
-	}
+		listener.Close()
 
-	var err error
-	if _, err = DialWithDialer(dialer, "tcp", addr, nil); err == nil {
-		t.Fatal("DialWithTimeout completed successfully")
-	}
+		// We're looking for a timeout during the handshake, so check that the
+		// Listener actually accepted the connection to initiate it. (If the server
+		// takes too long to accept the connection, we might cancel before the
+		// underlying net.Conn is ever dialed — without ever attempting a
+		// handshake.)
+		lconn, ok := <-acceptc
+		if ok {
+			// The Listener accepted a connection, so assume that it was from our
+			// Dial: we triggered the timeout at the point where we wanted it!
+			t.Logf("Listener accepted a connection from %s", lconn.RemoteAddr())
+			lconn.Close()
+		}
+		// Close any spurious extra connecitions from the listener. (This is
+		// possible if there are, for example, stray Dial calls from other tests.)
+		for extraConn := range acceptc {
+			t.Logf("spurious extra connection from %s", extraConn.RemoteAddr())
+			extraConn.Close()
+		}
+		if ok {
+			break
+		}
 
-	if !isTimeoutError(err) {
-		t.Errorf("resulting error not a timeout: %v\nType %T: %#v", err, err, err)
+		t.Logf("with timeout %v, DialWithDialer returned before listener accepted any connections; retrying", timeout)
+		timeout *= 2
 	}
 }