os/exec: fix fd leak with Std*Pipe + LookPath

If LookPath in Command fails, sets a sticky error, and then
StdinPipe, StdoutPipe, or StderrPipe were called, those pipe
fds were never cleaned up.

Fixes #5071

R=golang-dev, rogpeppe
CC=golang-dev
https://golang.org/cl/7799046

diff --git a/src/pkg/os/exec/exec.go b/src/pkg/os/exec/exec.go
index 8368491b0fef54b46bca40baea1f770736fd912a..a3bbcf3005a44d0fe6e7c7296431eacff23c77b6 100644 (file)
--- a/src/pkg/os/exec/exec.go
+++ b/src/pkg/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/src/pkg/os/exec/exec_test.go b/src/pkg/os/exec/exec_test.go
index 611ac0267669281d070ff836cd62a7965e1245b1..dfcf4be231456c42a5267b8e07854e1bc7dfd601 100644 (file)
--- a/src/pkg/os/exec/exec_test.go
+++ b/src/pkg/os/exec/exec_test.go
@@ -151,6 +151,33 @@ 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