all: fix incorrect channel and API usage in some unit tests

This CL changes some unit test functions, making sure that these tests (and goroutines spawned during test) won't block.
Since they are just test functions, I use one CL to fix them all. I hope this won't cause trouble to reviewers and can save time for us.
There are three main categories of incorrect logic fixed by this CL:
1. Use testing.Fatal()/Fatalf() in spawned goroutines, which is forbidden by Go's document.
2. Channels are used in such a way that, when errors or timeout happen, the test will be blocked and never return.
3. Channels are used in such a way that, when errors or timeout happen, the test can return but some spawned goroutines will be leaked, occupying resource until all other tests return and the process is killed.

Change-Id: I3df931ec380794a0cf1404e632c1dd57c65d63e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/219380
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

handshake_client_test.go

@@ -1748,7 +1748,7 @@ func TestHandshakeRace(t *testing.T) {
 
 		startWrite := make(chan struct{})
 		startRead := make(chan struct{})
-		readDone := make(chan struct{})
+		readDone := make(chan struct{}, 1)
 
 		client := Client(c, testConfig)
 		go func() {

handshake_server_test.go

@@ -182,7 +182,7 @@ func TestRenegotiationExtension(t *testing.T) {
 		cipherSuites:                 []uint16{TLS_RSA_WITH_RC4_128_SHA},
 	}
 
-	bufChan := make(chan []byte)
+	bufChan := make(chan []byte, 1)
 	c, s := localPipe(t)
 
 	go func() {
@@ -575,11 +575,12 @@ func (test *serverTest) connFromCommand() (conn *recordingConn, child *exec.Cmd,
 		return nil, nil, err
 	}
 
-	connChan := make(chan interface{})
+	connChan := make(chan interface{}, 1)
 	go func() {
 		tcpConn, err := l.Accept()
 		if err != nil {
 			connChan <- err
+			return
 		}
 		connChan <- tcpConn
 	}()

tls_test.go

@@ -294,7 +294,11 @@ func TestTLSUniqueMatches(t *testing.T) {
 	defer ln.Close()
 
 	serverTLSUniques := make(chan []byte)
+	parentDone := make(chan struct{})
+	childDone := make(chan struct{})
+	defer close(parentDone)
 	go func() {
+		defer close(childDone)
 		for i := 0; i < 2; i++ {
 			sconn, err := ln.Accept()
 			if err != nil {
@@ -308,7 +312,11 @@ func TestTLSUniqueMatches(t *testing.T) {
 				t.Error(err)
 				return
 			}
-			serverTLSUniques <- srv.ConnectionState().TLSUnique
+			select {
+			case <-parentDone:
+				return
+			case serverTLSUniques <- srv.ConnectionState().TLSUnique:
+			}
 		}
 	}()
 
@@ -318,7 +326,15 @@ func TestTLSUniqueMatches(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !bytes.Equal(conn.ConnectionState().TLSUnique, <-serverTLSUniques) {
+
+	var serverTLSUniquesValue []byte
+	select {
+	case <-childDone:
+		return
+	case serverTLSUniquesValue = <-serverTLSUniques:
+	}
+
+	if !bytes.Equal(conn.ConnectionState().TLSUnique, serverTLSUniquesValue) {
 		t.Error("client and server channel bindings differ")
 	}
 	conn.Close()
@@ -331,7 +347,14 @@ func TestTLSUniqueMatches(t *testing.T) {
 	if !conn.ConnectionState().DidResume {
 		t.Error("second session did not use resumption")
 	}
-	if !bytes.Equal(conn.ConnectionState().TLSUnique, <-serverTLSUniques) {
+
+	select {
+	case <-childDone:
+		return
+	case serverTLSUniquesValue = <-serverTLSUniques:
+	}
+
+	if !bytes.Equal(conn.ConnectionState().TLSUnique, serverTLSUniquesValue) {
 		t.Error("client and server channel bindings differ when session resumption is used")
 	}
 }