os/exec: Stdout/Stderr not closed after data copied

[tomlanyon]

Problem

os/exec: when command.Stdout or command.Stderr are provided an io.Writer which is not an *os.File, the goroutine that is created to copy output to the Writer never Closes that Writer.

This manifests unexpectedly in situations like the example below, where the bufio.Scanner blocks forever on a Read from an io.Pipe.

Passing a compress/gzip.Writer to command.Stdout/Stderr behaves similarly because its Close method needs to be called before the data and footer are written to its output.

The gzip.Writer case can be trivially worked around by explicitly calling its Close method after cmd.Wait, but is unintuitive. The io.Pipe example below is not easy to work around.

What version of Go are you using (go version)?

1.9
master

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

darwin/amd64
linux/amd64

What did you do?

package main

import (
        "bufio"
        "fmt"
        "io"
        "os/exec"
)

func main() {
        cmd := exec.Command("echo", "hello, world")

        pr, pw := io.Pipe()
        cmd.Stdout = pw
        cmd.Stderr = pw

        done := make(chan struct{})
        go func() {
                s := bufio.NewScanner(pr)
                for s.Scan() {
                        fmt.Printf("output: %v\n", s.Text())
                }
                done <- struct{}{}
        }()

        if err := cmd.Start(); err != nil {
                fmt.Println(err)
                return
        }

        // Wait for all output to be read before calling Wait.
        <-done
        fmt.Println("finished reading output")

        if err := cmd.Wait(); err != nil {
                fmt.Println(err)
        }
}

What did you expect to see?

% go run -race example.go
output: hello, world
finished reading output
<program terminates>

What did you see instead?

% go run -race example.go
output: hello, world
<program hangs indefinitely>

[gopherbot]

Change https://golang.org/cl/76320 mentions this issue: os/exec: attempt to Close command.Stdout/Stderr after copying data.

[tomlanyon]

Seems similar to #19685.

I've sent https://golang.org/cl/76320 which I feel like is an easy solution that makes the behaviour more closely match expectations.

[ianlancetaylor]

As noted on the CL, simply closing cmd.Stdout and cmd.Stderr can not be correct, as people often write cmd.Stdout = os.Stdout and we definitely do not want to close os.Stdout as a side-effect of executing such a command.