Commit 944c58e9 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

ipv4: deflake TestConnUnicastSocketOptions on Windows

TestConnUnicastSocketOptions doesn't work on nacl, plan9, or windows,
but the t.Skip at the top was missing the GOOS == "windows" check. (It
was present in the ipv6 version of the test)

The test than proceeded to try to run and set up a connection pair
for testing. Here's what was happening.

Goroutines are M (main, the client) and S (server, doing the Accept):

   M: net.Listen("tcp4", "127.0.0.1:0"), succeeds.
   M: defer ln.Close() enqueued.
   M: go acceptor() (start "S" (server) goroutine)
   M: net.Dial("tcp4", ln.Addr().String()), succeeds.
      Notably, it's connected to the kernel's TCP stack at this point.
      The userspace Accept hasn't run. Go's listen backlog is
      syscall.SOMAXCONN on Windows (which is fine).
   M: testUnicastSocketOptions:
      - finds that it's GOOS == "windows",
      - calls t.Skipf, which calls runtime.Goexit, runs deferred goroutines
   M: ln.Close() (previously deferred runs)
   M: test completes

Then:

   S: server goroutine finally schedules
   S: c, err := ln.Accept(), gets an error (because it's closed)
      if err != nil {
           t.Error(err)   // setting an error after test is done; bogus

So, fix it both ways: skip the test earlier on "windows", and simplify
the server goroutine and move the failing into the main goroutine.
Also skip other tests earlier on Windows that will also t.Skip. They
don't have goroutines, but might as well skip earlier.

Fixes golang/go#17655

Change-Id: I5d910d70943be37b2b4b56542ee98e42fc3acd71
Reviewed-on: https://go-review.googlesource.com/34021
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: 's avatarBryan Mills <bcmills@google.com>
parent de22433a
// Copyright 2012 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package ipv4_test
import (
"net"
"testing"
)
func acceptor(t *testing.T, ln net.Listener, done chan<- bool) {
defer func() { done <- true }()
c, err := ln.Accept()
if err != nil {
t.Error(err)
return
}
c.Close()
}
...@@ -16,7 +16,7 @@ import ( ...@@ -16,7 +16,7 @@ import (
func TestConnUnicastSocketOptions(t *testing.T) { func TestConnUnicastSocketOptions(t *testing.T) {
switch runtime.GOOS { switch runtime.GOOS {
case "nacl", "plan9": case "nacl", "plan9", "windows":
t.Skipf("not supported on %s", runtime.GOOS) t.Skipf("not supported on %s", runtime.GOOS)
} }
ifi := nettest.RoutedInterface("ip4", net.FlagUp|net.FlagLoopback) ifi := nettest.RoutedInterface("ip4", net.FlagUp|net.FlagLoopback)
...@@ -30,8 +30,15 @@ func TestConnUnicastSocketOptions(t *testing.T) { ...@@ -30,8 +30,15 @@ func TestConnUnicastSocketOptions(t *testing.T) {
} }
defer ln.Close() defer ln.Close()
done := make(chan bool) errc := make(chan error, 1)
go acceptor(t, ln, done) go func() {
c, err := ln.Accept()
if err != nil {
errc <- err
return
}
errc <- c.Close()
}()
c, err := net.Dial("tcp4", ln.Addr().String()) c, err := net.Dial("tcp4", ln.Addr().String())
if err != nil { if err != nil {
...@@ -41,7 +48,9 @@ func TestConnUnicastSocketOptions(t *testing.T) { ...@@ -41,7 +48,9 @@ func TestConnUnicastSocketOptions(t *testing.T) {
testUnicastSocketOptions(t, ipv4.NewConn(c)) testUnicastSocketOptions(t, ipv4.NewConn(c))
<-done if err := <-errc; err != nil {
t.Errorf("server: %v", err)
}
} }
var packetConnUnicastSocketOptionTests = []struct { var packetConnUnicastSocketOptionTests = []struct {
...@@ -53,7 +62,7 @@ var packetConnUnicastSocketOptionTests = []struct { ...@@ -53,7 +62,7 @@ var packetConnUnicastSocketOptionTests = []struct {
func TestPacketConnUnicastSocketOptions(t *testing.T) { func TestPacketConnUnicastSocketOptions(t *testing.T) {
switch runtime.GOOS { switch runtime.GOOS {
case "nacl", "plan9": case "nacl", "plan9", "windows":
t.Skipf("not supported on %s", runtime.GOOS) t.Skipf("not supported on %s", runtime.GOOS)
} }
ifi := nettest.RoutedInterface("ip4", net.FlagUp|net.FlagLoopback) ifi := nettest.RoutedInterface("ip4", net.FlagUp|net.FlagLoopback)
...@@ -79,7 +88,7 @@ func TestPacketConnUnicastSocketOptions(t *testing.T) { ...@@ -79,7 +88,7 @@ func TestPacketConnUnicastSocketOptions(t *testing.T) {
func TestRawConnUnicastSocketOptions(t *testing.T) { func TestRawConnUnicastSocketOptions(t *testing.T) {
switch runtime.GOOS { switch runtime.GOOS {
case "nacl", "plan9": case "nacl", "plan9", "windows":
t.Skipf("not supported on %s", runtime.GOOS) t.Skipf("not supported on %s", runtime.GOOS)
} }
if m, ok := nettest.SupportsRawIPSocket(); !ok { if m, ok := nettest.SupportsRawIPSocket(); !ok {
......
...@@ -29,8 +29,15 @@ func TestConnUnicastSocketOptions(t *testing.T) { ...@@ -29,8 +29,15 @@ func TestConnUnicastSocketOptions(t *testing.T) {
} }
defer ln.Close() defer ln.Close()
done := make(chan bool) errc := make(chan error, 1)
go acceptor(t, ln, done) go func() {
c, err := ln.Accept()
if err != nil {
errc <- err
return
}
errc <- c.Close()
}()
c, err := net.Dial("tcp6", ln.Addr().String()) c, err := net.Dial("tcp6", ln.Addr().String())
if err != nil { if err != nil {
...@@ -40,7 +47,9 @@ func TestConnUnicastSocketOptions(t *testing.T) { ...@@ -40,7 +47,9 @@ func TestConnUnicastSocketOptions(t *testing.T) {
testUnicastSocketOptions(t, ipv6.NewConn(c)) testUnicastSocketOptions(t, ipv6.NewConn(c))
<-done if err := <-errc; err != nil {
t.Errorf("server: %v", err)
}
} }
var packetConnUnicastSocketOptionTests = []struct { var packetConnUnicastSocketOptionTests = []struct {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment