diff options
Diffstat (limited to 'libgo/go/os/exec')
| -rw-r--r-- | libgo/go/os/exec/exec.go | 2 | ||||
| -rw-r--r-- | libgo/go/os/exec/exec_test.go | 206 | ||||
| -rw-r--r-- | libgo/go/os/exec/lp_plan9.go | 3 | ||||
| -rw-r--r-- | libgo/go/os/exec/lp_unix.go | 3 | ||||
| -rw-r--r-- | libgo/go/os/exec/lp_unix_test.go | 55 | ||||
| -rw-r--r-- | libgo/go/os/exec/lp_windows.go | 35 |
6 files changed, 290 insertions, 14 deletions
diff --git a/libgo/go/os/exec/exec.go b/libgo/go/os/exec/exec.go index 8368491b0fe..a3bbcf3005a 100644 --- a/libgo/go/os/exec/exec.go +++ b/libgo/go/os/exec/exec.go @@ -235,6 +235,8 @@ func (c *Cmd) Run() error { // Start starts the specified command but does not wait for it to complete. func (c *Cmd) Start() error { if c.err != nil { + c.closeDescriptors(c.closeAfterStart) + c.closeDescriptors(c.closeAfterWait) return c.err } if c.Process != nil { diff --git a/libgo/go/os/exec/exec_test.go b/libgo/go/os/exec/exec_test.go index ff8954fd020..fa7e88c6596 100644 --- a/libgo/go/os/exec/exec_test.go +++ b/libgo/go/os/exec/exec_test.go @@ -14,17 +14,23 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "runtime" "strconv" "strings" "testing" + "time" ) func helperCommand(s ...string) *Cmd { cs := []string{"-test.run=TestHelperProcess", "--"} cs = append(cs, s...) cmd := Command(os.Args[0], cs...) - cmd.Env = append([]string{"GO_WANT_HELPER_PROCESS=1"}, os.Environ()...) + cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"} + path := os.Getenv("LD_LIBRARY_PATH") + if path != "" { + cmd.Env = append(cmd.Env, "LD_LIBRARY_PATH="+path) + } return cmd } @@ -83,10 +89,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,8 +156,141 @@ func TestPipes(t *testing.T) { check("Wait", err) } +// Issue 5071 +func TestPipeLookPathLeak(t *testing.T) { + fd0 := numOpenFDS(t) + for i := 0; i < 4; i++ { + cmd := Command("something-that-does-not-exist-binary") + cmd.StdoutPipe() + cmd.StderrPipe() + cmd.StdinPipe() + if err := cmd.Run(); err == nil { + t.Fatal("unexpected success") + } + } + fdGrowth := numOpenFDS(t) - fd0 + if fdGrowth > 2 { + t.Errorf("leaked %d fds; want ~0", fdGrowth) + } +} + +func numOpenFDS(t *testing.T) int { + lsof, err := Command("lsof", "-n", "-p", strconv.Itoa(os.Getpid())).Output() + if err != nil { + t.Skip("skipping test; error finding or running lsof") + return 0 + } + return bytes.Count(lsof, []byte("\n")) +} + 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 TestExtraFilesFDShuffle(t *testing.T) { + t.Skip("TODO: TestExtraFilesFDShuffle is too non-portable; skipping") + + // syscall.StartProcess maps all the FDs passed to it in + // ProcAttr.Files (the concatenation of stdin,stdout,stderr and + // ExtraFiles) into consecutive FDs in the child, that is: + // Files{11, 12, 6, 7, 9, 3} should result in the file + // represented by FD 11 in the parent being made available as 0 + // in the child, 12 as 1, etc. + // + // We want to test that FDs in the child do not get overwritten + // by one another as this shuffle occurs. The original implementation + // was buggy in that in some data dependent cases it would ovewrite + // stderr in the child with one of the ExtraFile members. + // Testing for this case is difficult because it relies on using + // the same FD values as that case. In particular, an FD of 3 + // must be at an index of 4 or higher in ProcAttr.Files and + // the FD of the write end of the Stderr pipe (as obtained by + // StderrPipe()) must be the same as the size of ProcAttr.Files; + // therefore we test that the read end of this pipe (which is what + // is returned to the parent by StderrPipe() being one less than + // the size of ProcAttr.Files, i.e. 3+len(cmd.ExtraFiles). + // + // Moving this test case around within the overall tests may + // affect the FDs obtained and hence the checks to catch these cases. + npipes := 2 + c := helperCommand("extraFilesAndPipes", strconv.Itoa(npipes+1)) + rd, wr, _ := os.Pipe() + defer rd.Close() + if rd.Fd() != 3 { + t.Errorf("bad test value for test pipe: fd %d", rd.Fd()) + } + stderr, _ := c.StderrPipe() + wr.WriteString("_LAST") + wr.Close() + + pipes := make([]struct { + r, w *os.File + }, npipes) + data := []string{"a", "b"} + + for i := 0; i < npipes; i++ { + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("unexpected error creating pipe: %s", err) + } + pipes[i].r = r + pipes[i].w = w + w.WriteString(data[i]) + c.ExtraFiles = append(c.ExtraFiles, pipes[i].r) + defer func() { + r.Close() + w.Close() + }() + } + // Put fd 3 at the end. + c.ExtraFiles = append(c.ExtraFiles, rd) + + stderrFd := int(stderr.(*os.File).Fd()) + if stderrFd != ((len(c.ExtraFiles) + 3) - 1) { + t.Errorf("bad test value for stderr pipe") + } + + expected := "child: " + strings.Join(data, "") + "_LAST" + + err := c.Start() + if err != nil { + t.Fatalf("Run: %v", err) + } + ch := make(chan string, 1) + go func(ch chan string) { + buf := make([]byte, 512) + n, err := stderr.Read(buf) + if err != nil { + t.Fatalf("Read: %s", err) + ch <- err.Error() + } else { + ch <- string(buf[:n]) + } + close(ch) + }(ch) + select { + case m := <-ch: + if m != expected { + t.Errorf("Read: '%s' not '%s'", m, expected) + } + case <-time.After(5 * time.Second): + t.Errorf("Read timedout") + } + c.Wait() +} + func TestExtraFiles(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("no operating system support; skipping") @@ -155,7 +300,7 @@ func TestExtraFiles(t *testing.T) { // our environment. if !testedAlreadyLeaked { testedAlreadyLeaked = true - for fd := os.Stderr.Fd() + 1; fd <= 101; fd++ { + for fd := basefds(); fd <= 101; fd++ { err := os.NewFile(fd, "").Close() if err == nil { t.Logf("Something already leaked - closed fd %d", fd) @@ -209,13 +354,16 @@ 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) } } @@ -265,6 +413,13 @@ func TestExtraFilesRace(t *testing.T) { } la.Close() lb.Close() + for _, f := range ca.ExtraFiles { + f.Close() + } + for _, f := range cb.ExtraFiles { + f.Close() + } + } } @@ -360,7 +515,7 @@ func TestHelperProcess(*testing.T) { 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) @@ -384,7 +539,7 @@ 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) @@ -398,6 +553,35 @@ func TestHelperProcess(*testing.T) { } } os.Exit(0) + case "extraFilesAndPipes": + n, _ := strconv.Atoi(args[0]) + pipes := make([]*os.File, n) + for i := 0; i < n; i++ { + pipes[i] = os.NewFile(uintptr(3+i), strconv.Itoa(i)) + } + response := "" + for i, r := range pipes { + ch := make(chan string, 1) + go func(c chan string) { + buf := make([]byte, 10) + n, err := r.Read(buf) + if err != nil { + fmt.Fprintf(os.Stderr, "Child: read error: %v on pipe %d\n", err, i) + os.Exit(1) + } + c <- string(buf[:n]) + close(c) + }(ch) + select { + case m := <-ch: + response = response + m + case <-time.After(5 * time.Second): + fmt.Fprintf(os.Stderr, "Child: Timeout reading from pipe: %d\n", i) + os.Exit(1) + } + } + fmt.Fprintf(os.Stderr, "child: %s", response) + os.Exit(0) default: fmt.Fprintf(os.Stderr, "Unknown command %q\n", cmd) os.Exit(2) diff --git a/libgo/go/os/exec/lp_plan9.go b/libgo/go/os/exec/lp_plan9.go index 0e229e03ee7..6846a35c85f 100644 --- a/libgo/go/os/exec/lp_plan9.go +++ b/libgo/go/os/exec/lp_plan9.go @@ -8,7 +8,6 @@ import ( "errors" "os" "strings" - "syscall" ) // ErrNotFound is the error resulting if a path search failed to find an executable file. @@ -22,7 +21,7 @@ func findExecutable(file string) error { if m := d.Mode(); !m.IsDir() && m&0111 != 0 { return nil } - return syscall.EPERM + return os.ErrPermission } // LookPath searches for an executable binary named file diff --git a/libgo/go/os/exec/lp_unix.go b/libgo/go/os/exec/lp_unix.go index 21632219972..1d1ec07da4d 100644 --- a/libgo/go/os/exec/lp_unix.go +++ b/libgo/go/os/exec/lp_unix.go @@ -42,6 +42,9 @@ func LookPath(file string) (string, error) { return "", &Error{file, err} } pathenv := os.Getenv("PATH") + if pathenv == "" { + return "", &Error{file, ErrNotFound} + } for _, dir := range strings.Split(pathenv, ":") { if dir == "" { // Unix shell semantics: path element "" means "." diff --git a/libgo/go/os/exec/lp_unix_test.go b/libgo/go/os/exec/lp_unix_test.go new file mode 100644 index 00000000000..625d7848641 --- /dev/null +++ b/libgo/go/os/exec/lp_unix_test.go @@ -0,0 +1,55 @@ +// Copyright 2013 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. + +// +build darwin freebsd linux netbsd openbsd + +package exec + +import ( + "io/ioutil" + "os" + "testing" +) + +func TestLookPathUnixEmptyPath(t *testing.T) { + tmp, err := ioutil.TempDir("", "TestLookPathUnixEmptyPath") + if err != nil { + t.Fatal("TempDir failed: ", err) + } + defer os.RemoveAll(tmp) + wd, err := os.Getwd() + if err != nil { + t.Fatal("Getwd failed: ", err) + } + err = os.Chdir(tmp) + if err != nil { + t.Fatal("Chdir failed: ", err) + } + defer os.Chdir(wd) + + f, err := os.OpenFile("exec_me", os.O_CREATE|os.O_EXCL, 0700) + if err != nil { + t.Fatal("OpenFile failed: ", err) + } + err = f.Close() + if err != nil { + t.Fatal("Close failed: ", err) + } + + pathenv := os.Getenv("PATH") + defer os.Setenv("PATH", pathenv) + + err = os.Setenv("PATH", "") + if err != nil { + t.Fatal("Setenv failed: ", err) + } + + path, err := LookPath("exec_me") + if err == nil { + t.Fatal("LookPath found exec_me in empty $PATH") + } + if path != "" { + t.Fatalf("LookPath path == %q when err != nil", path) + } +} diff --git a/libgo/go/os/exec/lp_windows.go b/libgo/go/os/exec/lp_windows.go index d8351d7e6d3..7c7289bceea 100644 --- a/libgo/go/os/exec/lp_windows.go +++ b/libgo/go/os/exec/lp_windows.go @@ -72,7 +72,7 @@ func LookPath(file string) (f string, err error) { return } if pathenv := os.Getenv(`PATH`); pathenv != `` { - for _, dir := range strings.Split(pathenv, `;`) { + for _, dir := range splitList(pathenv) { if f, err = findExecutable(dir+`\`+file, exts); err == nil { return } @@ -80,3 +80,36 @@ func LookPath(file string) (f string, err error) { } return ``, &Error{file, ErrNotFound} } + +func splitList(path string) []string { + // The same implementation is used in SplitList in path/filepath; + // consider changing path/filepath when changing this. + + if path == "" { + return []string{} + } + + // Split path, respecting but preserving quotes. + list := []string{} + start := 0 + quo := false + for i := 0; i < len(path); i++ { + switch c := path[i]; { + case c == '"': + quo = !quo + case c == os.PathListSeparator && !quo: + list = append(list, path[start:i]) + start = i + 1 + } + } + list = append(list, path[start:]) + + // Remove quotes. + for i, s := range list { + if strings.Contains(s, `"`) { + list[i] = strings.Replace(s, `"`, ``, -1) + } + } + + return list +} |