1 files changed, 119 insertions, 15 deletions
diff --git a/src/pkg/os/exec/exec_test.go b/src/pkg/os/exec/exec_test.go
index 52f4bce3a..611ac0267 100644
--- a/src/pkg/os/exec/exec_test.go
+++ b/src/pkg/os/exec/exec_test.go
@@ -14,6 +14,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strconv"
 	"strings"
@@ -83,10 +84,16 @@ func TestNoExistBinary(t *testing.T) {
 
 func TestExitStatus(t *testing.T) {
 	// Test that exit values are returned correctly
-	err := helperCommand("exit", "42").Run()
+	cmd := helperCommand("exit", "42")
+	err := cmd.Run()
+	want := "exit status 42"
+	switch runtime.GOOS {
+	case "plan9":
+		want = fmt.Sprintf("exit status: '%s %d: 42'", filepath.Base(cmd.Path), cmd.ProcessState.Pid())
+	}
 	if werr, ok := err.(*ExitError); ok {
-		if s, e := werr.Error(), "exit status 42"; s != e {
-			t.Errorf("from exit 42 got exit %q, want %q", s, e)
+		if s := werr.Error(); s != want {
+			t.Errorf("from exit 42 got exit %q, want %q", s, want)
 		}
 	} else {
 		t.Fatalf("expected *ExitError from exit 42; got %T: %v", err, err)
@@ -144,18 +151,36 @@ func TestPipes(t *testing.T) {
 	check("Wait", err)
 }
 
+var testedAlreadyLeaked = false
+
+// basefds returns the number of expected file descriptors
+// to be present in a process at start.
+func basefds() uintptr {
+	n := os.Stderr.Fd() + 1
+
+	// Go runtime for 32-bit Plan 9 requires that /dev/bintime
+	// be kept open.
+	// See ../../runtime/time_plan9_386.c:/^runtime·nanotime
+	if runtime.GOOS == "plan9" && runtime.GOARCH == "386" {
+		n++
+	}
+	return n
+}
+
 func TestExtraFiles(t *testing.T) {
 	if runtime.GOOS == "windows" {
-		t.Logf("no operating system support; skipping")
-		return
+		t.Skip("no operating system support; skipping")
 	}
 
 	// Ensure that file descriptors have not already been leaked into
 	// our environment.
-	for fd := os.Stderr.Fd() + 1; fd <= 101; fd++ {
-		err := os.NewFile(fd, "").Close()
-		if err == nil {
-			t.Logf("Something already leaked - closed fd %d", fd)
+	if !testedAlreadyLeaked {
+		testedAlreadyLeaked = true
+		for fd := basefds(); fd <= 101; fd++ {
+			err := os.NewFile(fd, "").Close()
+			if err == nil {
+				t.Logf("Something already leaked - closed fd %d", fd)
+			}
 		}
 	}
 
@@ -167,6 +192,18 @@ func TestExtraFiles(t *testing.T) {
 	}
 	defer ln.Close()
 
+	// Make sure duplicated fds don't leak to the child.
+	f, err := ln.(*net.TCPListener).File()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+	ln2, err := net.FileListener(f)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ln2.Close()
+
 	// Force TLS root certs to be loaded (which might involve
 	// cgo), to make sure none of that potential C code leaks fds.
 	ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -193,13 +230,65 @@ func TestExtraFiles(t *testing.T) {
 	}
 
 	c := helperCommand("read3")
+	var stdout, stderr bytes.Buffer
+	c.Stdout = &stdout
+	c.Stderr = &stderr
 	c.ExtraFiles = []*os.File{tf}
-	bs, err := c.CombinedOutput()
+	err = c.Run()
 	if err != nil {
-		t.Fatalf("CombinedOutput: %v; output %q", err, bs)
+		t.Fatalf("Run: %v; stdout %q, stderr %q", err, stdout.Bytes(), stderr.Bytes())
 	}
-	if string(bs) != text {
-		t.Errorf("got %q; want %q", string(bs), text)
+	if stdout.String() != text {
+		t.Errorf("got stdout %q, stderr %q; want %q on stdout", stdout.String(), stderr.String(), text)
+	}
+}
+
+func TestExtraFilesRace(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("no operating system support; skipping")
+	}
+	listen := func() net.Listener {
+		ln, err := net.Listen("tcp", "127.0.0.1:0")
+		if err != nil {
+			t.Fatal(err)
+		}
+		return ln
+	}
+	listenerFile := func(ln net.Listener) *os.File {
+		f, err := ln.(*net.TCPListener).File()
+		if err != nil {
+			t.Fatal(err)
+		}
+		return f
+	}
+	runCommand := func(c *Cmd, out chan<- string) {
+		bout, err := c.CombinedOutput()
+		if err != nil {
+			out <- "ERROR:" + err.Error()
+		} else {
+			out <- string(bout)
+		}
+	}
+
+	for i := 0; i < 10; i++ {
+		la := listen()
+		ca := helperCommand("describefiles")
+		ca.ExtraFiles = []*os.File{listenerFile(la)}
+		lb := listen()
+		cb := helperCommand("describefiles")
+		cb.ExtraFiles = []*os.File{listenerFile(lb)}
+		ares := make(chan string)
+		bres := make(chan string)
+		go runCommand(ca, ares)
+		go runCommand(cb, bres)
+		if got, want := <-ares, fmt.Sprintf("fd3: listener %s\n", la.Addr()); got != want {
+			t.Errorf("iteration %d, process A got:\n%s\nwant:\n%s\n", i, got, want)
+		}
+		if got, want := <-bres, fmt.Sprintf("fd3: listener %s\n", lb.Addr()); got != want {
+			t.Errorf("iteration %d, process B got:\n%s\nwant:\n%s\n", i, got, want)
+		}
+		la.Close()
+		lb.Close()
 	}
 }
 
@@ -287,10 +376,15 @@ func TestHelperProcess(*testing.T) {
 			// TODO(bradfitz): broken? Sometimes.
 			// http://golang.org/issue/2603
 			// Skip this additional part of the test for now.
+		case "netbsd":
+			// TODO(jsing): This currently fails on NetBSD due to
+			// the cloned file descriptors that result from opening
+			// /dev/urandom.
+			// http://golang.org/issue/3955
 		default:
 			// Now verify that there are no other open fds.
 			var files []*os.File
-			for wantfd := os.Stderr.Fd() + 2; wantfd <= 100; wantfd++ {
+			for wantfd := basefds() + 1; wantfd <= 100; wantfd++ {
 				f, err := os.Open(os.Args[0])
 				if err != nil {
 					fmt.Printf("error opening file with expected fd %d: %v", wantfd, err)
@@ -314,10 +408,20 @@ func TestHelperProcess(*testing.T) {
 		// what we do with fd3 as long as we refer to it;
 		// closing it is the easy choice.
 		fd3.Close()
-		os.Stderr.Write(bs)
+		os.Stdout.Write(bs)
 	case "exit":
 		n, _ := strconv.Atoi(args[0])
 		os.Exit(n)
+	case "describefiles":
+		for fd := uintptr(3); fd < 25; fd++ {
+			f := os.NewFile(fd, fmt.Sprintf("fd-%d", fd))
+			ln, err := net.FileListener(f)
+			if err == nil {
+				fmt.Printf("fd%d: listener %s\n", fd, ln.Addr())
+				ln.Close()
+			}
+		}
+		os.Exit(0)
 	default:
 		fmt.Fprintf(os.Stderr, "Unknown command %q\n", cmd)
 		os.Exit(2)