From c1f0edae04dccd509618ab8b594d18cb2b0a49f7 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 8 Apr 2020 12:51:50 -0400 Subject: [PATCH] net: convert many Close tests to use parallel subtests Also set a deadline in TestCloseWrite so that we can more easily determine which kind of connection is getting stuck on the darwin-arm64-corellium builder (#34837). Change-Id: I8ccacbf436e8e493fb2298a79b17e0af8fc6eb81 Reviewed-on: https://go-review.googlesource.com/c/go/+/227588 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/net/net_test.go | 568 +++++++++++++++++++++++--------------------- 1 file changed, 302 insertions(+), 266 deletions(-) diff --git a/src/net/net_test.go b/src/net/net_test.go index a7406740f5..409e1400af 100644 --- a/src/net/net_test.go +++ b/src/net/net_test.go @@ -23,50 +23,54 @@ func TestCloseRead(t *testing.T) { case "plan9": t.Skipf("not supported on %s", runtime.GOOS) } + t.Parallel() for _, network := range []string{"tcp", "unix", "unixpacket"} { - if !testableNetwork(network) { - t.Logf("skipping %s test", network) - continue - } - - ln, err := newLocalListener(network) - if err != nil { - t.Fatal(err) - } - switch network { - case "unix", "unixpacket": - defer os.Remove(ln.Addr().String()) - } - defer ln.Close() - - c, err := Dial(ln.Addr().Network(), ln.Addr().String()) - if err != nil { - t.Fatal(err) - } - switch network { - case "unix", "unixpacket": - defer os.Remove(c.LocalAddr().String()) - } - defer c.Close() - - switch c := c.(type) { - case *TCPConn: - err = c.CloseRead() - case *UnixConn: - err = c.CloseRead() - } - if err != nil { - if perr := parseCloseError(err, true); perr != nil { - t.Error(perr) + network := network + t.Run(network, func(t *testing.T) { + if !testableNetwork(network) { + t.Skipf("network %s is not testable on the current platform", network) } - t.Fatal(err) - } - var b [1]byte - n, err := c.Read(b[:]) - if n != 0 || err == nil { - t.Fatalf("got (%d, %v); want (0, error)", n, err) - } + t.Parallel() + + ln, err := newLocalListener(network) + if err != nil { + t.Fatal(err) + } + switch network { + case "unix", "unixpacket": + defer os.Remove(ln.Addr().String()) + } + defer ln.Close() + + c, err := Dial(ln.Addr().Network(), ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + switch network { + case "unix", "unixpacket": + defer os.Remove(c.LocalAddr().String()) + } + defer c.Close() + + switch c := c.(type) { + case *TCPConn: + err = c.CloseRead() + case *UnixConn: + err = c.CloseRead() + } + if err != nil { + if perr := parseCloseError(err, true); perr != nil { + t.Error(perr) + } + t.Fatal(err) + } + var b [1]byte + n, err := c.Read(b[:]) + if n != 0 || err == nil { + t.Fatalf("got (%d, %v); want (0, error)", n, err) + } + }) } } @@ -76,212 +80,240 @@ func TestCloseWrite(t *testing.T) { t.Skipf("not supported on %s", runtime.GOOS) } - handler := func(ls *localServer, ln Listener) { - c, err := ln.Accept() - if err != nil { - t.Error(err) - return - } - defer c.Close() - - var b [1]byte - n, err := c.Read(b[:]) - if n != 0 || err != io.EOF { - t.Errorf("got (%d, %v); want (0, io.EOF)", n, err) - return - } - switch c := c.(type) { - case *TCPConn: - err = c.CloseWrite() - case *UnixConn: - err = c.CloseWrite() - } - if err != nil { - if perr := parseCloseError(err, true); perr != nil { - t.Error(perr) - } - t.Error(err) - return - } - n, err = c.Write(b[:]) - if err == nil { - t.Errorf("got (%d, %v); want (any, error)", n, err) - return - } + t.Parallel() + deadline, _ := t.Deadline() + if !deadline.IsZero() { + // Leave 10% headroom on the deadline to report errors and clean up. + deadline = deadline.Add(-time.Until(deadline) / 10) } for _, network := range []string{"tcp", "unix", "unixpacket"} { - if !testableNetwork(network) { - t.Logf("skipping %s test", network) - continue - } - - ls, err := newLocalServer(network) - if err != nil { - t.Fatal(err) - } - defer ls.teardown() - if err := ls.buildup(handler); err != nil { - t.Fatal(err) - } - - c, err := Dial(ls.Listener.Addr().Network(), ls.Listener.Addr().String()) - if err != nil { - t.Fatal(err) - } - switch network { - case "unix", "unixpacket": - defer os.Remove(c.LocalAddr().String()) - } - defer c.Close() - - switch c := c.(type) { - case *TCPConn: - err = c.CloseWrite() - case *UnixConn: - err = c.CloseWrite() - } - if err != nil { - if perr := parseCloseError(err, true); perr != nil { - t.Error(perr) + network := network + t.Run(network, func(t *testing.T) { + if !testableNetwork(network) { + t.Skipf("network %s is not testable on the current platform", network) } - t.Fatal(err) - } - var b [1]byte - n, err := c.Read(b[:]) - if n != 0 || err != io.EOF { - t.Fatalf("got (%d, %v); want (0, io.EOF)", n, err) - } - n, err = c.Write(b[:]) - if err == nil { - t.Fatalf("got (%d, %v); want (any, error)", n, err) - } + t.Parallel() + + handler := func(ls *localServer, ln Listener) { + c, err := ln.Accept() + if err != nil { + t.Error(err) + return + } + if !deadline.IsZero() { + c.SetDeadline(deadline) + } + defer c.Close() + + var b [1]byte + n, err := c.Read(b[:]) + if n != 0 || err != io.EOF { + t.Errorf("got (%d, %v); want (0, io.EOF)", n, err) + return + } + switch c := c.(type) { + case *TCPConn: + err = c.CloseWrite() + case *UnixConn: + err = c.CloseWrite() + } + if err != nil { + if perr := parseCloseError(err, true); perr != nil { + t.Error(perr) + } + t.Error(err) + return + } + n, err = c.Write(b[:]) + if err == nil { + t.Errorf("got (%d, %v); want (any, error)", n, err) + return + } + } + + ls, err := newLocalServer(network) + if err != nil { + t.Fatal(err) + } + defer ls.teardown() + if err := ls.buildup(handler); err != nil { + t.Fatal(err) + } + + c, err := Dial(ls.Listener.Addr().Network(), ls.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + if !deadline.IsZero() { + c.SetDeadline(deadline) + } + switch network { + case "unix", "unixpacket": + defer os.Remove(c.LocalAddr().String()) + } + defer c.Close() + + switch c := c.(type) { + case *TCPConn: + err = c.CloseWrite() + case *UnixConn: + err = c.CloseWrite() + } + if err != nil { + if perr := parseCloseError(err, true); perr != nil { + t.Error(perr) + } + t.Fatal(err) + } + var b [1]byte + n, err := c.Read(b[:]) + if n != 0 || err != io.EOF { + t.Fatalf("got (%d, %v); want (0, io.EOF)", n, err) + } + n, err = c.Write(b[:]) + if err == nil { + t.Fatalf("got (%d, %v); want (any, error)", n, err) + } + }) } } func TestConnClose(t *testing.T) { + t.Parallel() for _, network := range []string{"tcp", "unix", "unixpacket"} { - if !testableNetwork(network) { - t.Logf("skipping %s test", network) - continue - } - - ln, err := newLocalListener(network) - if err != nil { - t.Fatal(err) - } - switch network { - case "unix", "unixpacket": - defer os.Remove(ln.Addr().String()) - } - defer ln.Close() - - c, err := Dial(ln.Addr().Network(), ln.Addr().String()) - if err != nil { - t.Fatal(err) - } - switch network { - case "unix", "unixpacket": - defer os.Remove(c.LocalAddr().String()) - } - defer c.Close() - - if err := c.Close(); err != nil { - if perr := parseCloseError(err, false); perr != nil { - t.Error(perr) + network := network + t.Run(network, func(t *testing.T) { + if !testableNetwork(network) { + t.Skipf("network %s is not testable on the current platform", network) } - t.Fatal(err) - } - var b [1]byte - n, err := c.Read(b[:]) - if n != 0 || err == nil { - t.Fatalf("got (%d, %v); want (0, error)", n, err) - } + t.Parallel() + + ln, err := newLocalListener(network) + if err != nil { + t.Fatal(err) + } + switch network { + case "unix", "unixpacket": + defer os.Remove(ln.Addr().String()) + } + defer ln.Close() + + c, err := Dial(ln.Addr().Network(), ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + switch network { + case "unix", "unixpacket": + defer os.Remove(c.LocalAddr().String()) + } + defer c.Close() + + if err := c.Close(); err != nil { + if perr := parseCloseError(err, false); perr != nil { + t.Error(perr) + } + t.Fatal(err) + } + var b [1]byte + n, err := c.Read(b[:]) + if n != 0 || err == nil { + t.Fatalf("got (%d, %v); want (0, error)", n, err) + } + }) } } func TestListenerClose(t *testing.T) { + t.Parallel() for _, network := range []string{"tcp", "unix", "unixpacket"} { - if !testableNetwork(network) { - t.Logf("skipping %s test", network) - continue - } - - ln, err := newLocalListener(network) - if err != nil { - t.Fatal(err) - } - switch network { - case "unix", "unixpacket": - defer os.Remove(ln.Addr().String()) - } - - dst := ln.Addr().String() - if err := ln.Close(); err != nil { - if perr := parseCloseError(err, false); perr != nil { - t.Error(perr) + network := network + t.Run(network, func(t *testing.T) { + if !testableNetwork(network) { + t.Skipf("network %s is not testable on the current platform", network) } - t.Fatal(err) - } - c, err := ln.Accept() - if err == nil { - c.Close() - t.Fatal("should fail") - } + t.Parallel() - if network == "tcp" { - // We will have two TCP FSMs inside the - // kernel here. There's no guarantee that a - // signal comes from the far end FSM will be - // delivered immediately to the near end FSM, - // especially on the platforms that allow - // multiple consumer threads to pull pending - // established connections at the same time by - // enabling SO_REUSEPORT option such as Linux, - // DragonFly BSD. So we need to give some time - // quantum to the kernel. - // - // Note that net.inet.tcp.reuseport_ext=1 by - // default on DragonFly BSD. - time.Sleep(time.Millisecond) + ln, err := newLocalListener(network) + if err != nil { + t.Fatal(err) + } + switch network { + case "unix", "unixpacket": + defer os.Remove(ln.Addr().String()) + } - cc, err := Dial("tcp", dst) + dst := ln.Addr().String() + if err := ln.Close(); err != nil { + if perr := parseCloseError(err, false); perr != nil { + t.Error(perr) + } + t.Fatal(err) + } + c, err := ln.Accept() if err == nil { - t.Error("Dial to closed TCP listener succeeded.") - cc.Close() + c.Close() + t.Fatal("should fail") } - } + + if network == "tcp" { + // We will have two TCP FSMs inside the + // kernel here. There's no guarantee that a + // signal comes from the far end FSM will be + // delivered immediately to the near end FSM, + // especially on the platforms that allow + // multiple consumer threads to pull pending + // established connections at the same time by + // enabling SO_REUSEPORT option such as Linux, + // DragonFly BSD. So we need to give some time + // quantum to the kernel. + // + // Note that net.inet.tcp.reuseport_ext=1 by + // default on DragonFly BSD. + time.Sleep(time.Millisecond) + + cc, err := Dial("tcp", dst) + if err == nil { + t.Error("Dial to closed TCP listener succeeded.") + cc.Close() + } + } + }) } } func TestPacketConnClose(t *testing.T) { + t.Parallel() for _, network := range []string{"udp", "unixgram"} { - if !testableNetwork(network) { - t.Logf("skipping %s test", network) - continue - } - - c, err := newLocalPacketListener(network) - if err != nil { - t.Fatal(err) - } - switch network { - case "unixgram": - defer os.Remove(c.LocalAddr().String()) - } - defer c.Close() - - if err := c.Close(); err != nil { - if perr := parseCloseError(err, false); perr != nil { - t.Error(perr) + network := network + t.Run(network, func(t *testing.T) { + if !testableNetwork(network) { + t.Skipf("network %s is not testable on the current platform", network) } - t.Fatal(err) - } - var b [1]byte - n, _, err := c.ReadFrom(b[:]) - if n != 0 || err == nil { - t.Fatalf("got (%d, %v); want (0, error)", n, err) - } + t.Parallel() + + c, err := newLocalPacketListener(network) + if err != nil { + t.Fatal(err) + } + switch network { + case "unixgram": + defer os.Remove(c.LocalAddr().String()) + } + defer c.Close() + + if err := c.Close(); err != nil { + if perr := parseCloseError(err, false); perr != nil { + t.Error(perr) + } + t.Fatal(err) + } + var b [1]byte + n, _, err := c.ReadFrom(b[:]) + if n != 0 || err == nil { + t.Fatalf("got (%d, %v); want (0, error)", n, err) + } + }) } } @@ -366,56 +398,60 @@ func TestAcceptIgnoreAbortedConnRequest(t *testing.T) { } func TestZeroByteRead(t *testing.T) { + t.Parallel() for _, network := range []string{"tcp", "unix", "unixpacket"} { - if !testableNetwork(network) { - t.Logf("skipping %s test", network) - continue - } - - ln, err := newLocalListener(network) - if err != nil { - t.Fatal(err) - } - connc := make(chan Conn, 1) - go func() { - defer ln.Close() - c, err := ln.Accept() - if err != nil { - t.Error(err) + network := network + t.Run(network, func(t *testing.T) { + if !testableNetwork(network) { + t.Skipf("network %s is not testable on the current platform", network) } - connc <- c // might be nil - }() - c, err := Dial(network, ln.Addr().String()) - if err != nil { - t.Fatal(err) - } - defer c.Close() - sc := <-connc - if sc == nil { - continue - } - defer sc.Close() + t.Parallel() - if runtime.GOOS == "windows" { - // A zero byte read on Windows caused a wait for readability first. - // Rather than change that behavior, satisfy it in this test. - // See Issue 15735. - go io.WriteString(sc, "a") - } + ln, err := newLocalListener(network) + if err != nil { + t.Fatal(err) + } + connc := make(chan Conn, 1) + go func() { + defer ln.Close() + c, err := ln.Accept() + if err != nil { + t.Error(err) + } + connc <- c // might be nil + }() + c, err := Dial(network, ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer c.Close() + sc := <-connc + if sc == nil { + return + } + defer sc.Close() - n, err := c.Read(nil) - if n != 0 || err != nil { - t.Errorf("%s: zero byte client read = %v, %v; want 0, nil", network, n, err) - } + if runtime.GOOS == "windows" { + // A zero byte read on Windows caused a wait for readability first. + // Rather than change that behavior, satisfy it in this test. + // See Issue 15735. + go io.WriteString(sc, "a") + } - if runtime.GOOS == "windows" { - // Same as comment above. - go io.WriteString(c, "a") - } - n, err = sc.Read(nil) - if n != 0 || err != nil { - t.Errorf("%s: zero byte server read = %v, %v; want 0, nil", network, n, err) - } + n, err := c.Read(nil) + if n != 0 || err != nil { + t.Errorf("%s: zero byte client read = %v, %v; want 0, nil", network, n, err) + } + + if runtime.GOOS == "windows" { + // Same as comment above. + go io.WriteString(c, "a") + } + n, err = sc.Read(nil) + if n != 0 || err != nil { + t.Errorf("%s: zero byte server read = %v, %v; want 0, nil", network, n, err) + } + }) } }