From: Bryan C. Mills Date: Tue, 23 Nov 2021 22:02:10 +0000 (-0500) Subject: log/syslog: create unix sockets in unique directories X-Git-Tag: go1.18beta1~78 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=6180c4f5ebae4635377dfa778e05097cf8fc69a8;p=gostls13.git 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 Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor --- 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) }