log/syslog: create unix sockets in unique directories

startServer was invoking os.Remove on the temporary file for a unix
socket after creating it. Since the files were created in the global
temp directory, that could cause two tests to arrive at colliding
names.

(Noticed while looking into the failure at
https://storage.googleapis.com/go-build-log/af2c83b1/solaris-amd64-oraclerel_3e01fda8.log,
but I would be surprised if this solves that failure.)

This change uses unique temporary directories, and attempts to keep
name lengths minimal to avoid accidentally running into socket-name
length limitations.

Updates #34611

Change-Id: I21743f245e5c14645e03f09795013e058b984471
Reviewed-on: https://go-review.googlesource.com/c/go/+/366774
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

1 files changed, 55 insertions, 39 deletions

diff --git a/src/log/syslog/syslog_test.go b/src/log/syslog/syslog_test.go
index 26530480ee..de1681d653 100644
--- a/src/log/syslog/syslog_test.go
+++ b/src/log/syslog/syslog_test.go
@@ -10,9 +10,9 @@ import (
 	"bufio"
 	"fmt"
 	"io"
-	"log"
 	"net"
 	"os"
+	"path/filepath"
 	"runtime"
 	"sync"
 	"testing"
@@ -81,28 +81,36 @@ func runStreamSyslog(l net.Listener, done chan<- string, wg *sync.WaitGroup) {
 	}
 }
 
-func startServer(n, la string, done chan<- string) (addr string, sock io.Closer, wg *sync.WaitGroup) {
+func startServer(t *testing.T, n, la string, done chan<- string) (addr string, sock io.Closer, wg *sync.WaitGroup) {
 	if n == "udp" || n == "tcp" {
 		la = "127.0.0.1:0"
 	} else {
-		// unix and unixgram: choose an address if none given
+		// unix and unixgram: choose an address if none given.
 		if la == "" {
-			// use os.CreateTemp to get a name that is unique
-			f, err := os.CreateTemp("", "syslogtest")
+			// The address must be short to fit in the sun_path field of the
+			// sockaddr_un passed to the underlying system calls, so we use
+			// os.MkdirTemp instead of t.TempDir: t.TempDir generally includes all or
+			// part of the test name in the directory, which can be much more verbose
+			// and risks running up against the limit.
+			dir, err := os.MkdirTemp("", "")
 			if err != nil {
-				log.Fatal("TempFile: ", err)
+				t.Fatal(err)
 			}
-			f.Close()
-			la = f.Name()
+			t.Cleanup(func() {
+				if err := os.RemoveAll(dir); err != nil {
+					t.Errorf("failed to remove socket temp directory: %v", err)
+				}
+			})
+			la = filepath.Join(dir, "sock")
 		}
-		os.Remove(la)
 	}
 
 	wg = new(sync.WaitGroup)
 	if n == "udp" || n == "unixgram" {
 		l, e := net.ListenPacket(n, la)
 		if e != nil {
-			log.Fatalf("startServer failed: %v", e)
+			t.Helper()
+			t.Fatalf("startServer failed: %v", e)
 		}
 		addr = l.LocalAddr().String()
 		sock = l
@@ -114,7 +122,8 @@ func startServer(n, la string, done chan<- string) (addr string, sock io.Closer,
 	} else {
 		l, e := net.Listen(n, la)
 		if e != nil {
-			log.Fatalf("startServer failed: %v", e)
+			t.Helper()
+			t.Fatalf("startServer failed: %v", e)
 		}
 		addr = l.Addr().String()
 		sock = l
@@ -129,32 +138,35 @@ func startServer(n, la string, done chan<- string) (addr string, sock io.Closer,
 
 func TestWithSimulated(t *testing.T) {
 	t.Parallel()
+
 	msg := "Test 123"
-	var transport []string
-	for _, n := range []string{"unix", "unixgram", "udp", "tcp"} {
-		if testableNetwork(n) {
-			transport = append(transport, n)
+	for _, tr := range []string{"unix", "unixgram", "udp", "tcp"} {
+		if !testableNetwork(tr) {
+			continue
 		}
-	}
 
-	for _, tr := range transport {
-		done := make(chan string)
-		addr, sock, srvWG := startServer(tr, "", done)
-		defer srvWG.Wait()
-		defer sock.Close()
-		if tr == "unix" || tr == "unixgram" {
-			defer os.Remove(addr)
-		}
-		s, err := Dial(tr, addr, LOG_INFO|LOG_USER, "syslog_test")
-		if err != nil {
-			t.Fatalf("Dial() failed: %v", err)
-		}
-		err = s.Info(msg)
-		if err != nil {
-			t.Fatalf("log failed: %v", err)
-		}
-		check(t, msg, <-done, tr)
-		s.Close()
+		tr := tr
+		t.Run(tr, func(t *testing.T) {
+			t.Parallel()
+
+			done := make(chan string)
+			addr, sock, srvWG := startServer(t, tr, "", done)
+			defer srvWG.Wait()
+			defer sock.Close()
+			if tr == "unix" || tr == "unixgram" {
+				defer os.Remove(addr)
+			}
+			s, err := Dial(tr, addr, LOG_INFO|LOG_USER, "syslog_test")
+			if err != nil {
+				t.Fatalf("Dial() failed: %v", err)
+			}
+			err = s.Info(msg)
+			if err != nil {
+				t.Fatalf("log failed: %v", err)
+			}
+			check(t, msg, <-done, tr)
+			s.Close()
+		})
 	}
 }
 
@@ -165,7 +177,7 @@ func TestFlap(t *testing.T) {
 	}
 
 	done := make(chan string)
-	addr, sock, srvWG := startServer(net, "", done)
+	addr, sock, srvWG := startServer(t, net, "", done)
 	defer srvWG.Wait()
 	defer os.Remove(addr)
 	defer sock.Close()
@@ -182,7 +194,10 @@ func TestFlap(t *testing.T) {
 	check(t, msg, <-done, net)
 
 	// restart the server
-	_, sock2, srvWG2 := startServer(net, addr, done)
+	if err := os.Remove(addr); err != nil {
+		t.Fatal(err)
+	}
+	_, sock2, srvWG2 := startServer(t, net, addr, done)
 	defer srvWG2.Wait()
 	defer sock2.Close()
 
@@ -282,6 +297,7 @@ func check(t *testing.T, in, out, transport string) {
 
 func TestWrite(t *testing.T) {
 	t.Parallel()
+
 	tests := []struct {
 		pri Priority
 		pre string
@@ -299,7 +315,7 @@ func TestWrite(t *testing.T) {
 	} else {
 		for _, test := range tests {
 			done := make(chan string)
-			addr, sock, srvWG := startServer("udp", "", done)
+			addr, sock, srvWG := startServer(t, "udp", "", done)
 			defer srvWG.Wait()
 			defer sock.Close()
 			l, err := Dial("udp", addr, test.pri, test.pre)
@@ -323,7 +339,7 @@ func TestWrite(t *testing.T) {
 }
 
 func TestConcurrentWrite(t *testing.T) {
-	addr, sock, srvWG := startServer("udp", "", make(chan string, 1))
+	addr, sock, srvWG := startServer(t, "udp", "", make(chan string, 1))
 	defer srvWG.Wait()
 	defer sock.Close()
 	w, err := Dial("udp", addr, LOG_USER|LOG_ERR, "how's it going?")
@@ -359,7 +375,7 @@ func TestConcurrentReconnect(t *testing.T) {
 		}
 	}
 	done := make(chan string, N*M)
-	addr, sock, srvWG := startServer(net, "", done)
+	addr, sock, srvWG := startServer(t, net, "", done)
 	if net == "unix" {
 		defer os.Remove(addr)
 	}