os/exec: make StdoutPipe and StderrPipe safe to Close concurrently

For #50436, I want to be able to close the pipes returned by
StdoutPipe and StderrPipe after the Context has been canceled
and the WaitDelay has subsequently expired.

However, the fact that the exec.onceCloser wrapper for StdinPipe
(added in CL 13329043) was retained in CL 65490 suggests to me that
(*os.File).Close is still not safe to call concurrently.

This may cause type assertions of these ReadClosers to *os.File that
once succeeded to no longer do so. However, the StdoutPipe and
StderrPipe methods return interfaces, not concrete *os.Files, so
callers already should not have been relying on that implementation
detail — and as far as I can tell the closeOnce wrapper does not mask
any (*os.File) methods, so assertions to any interface type that
previously succeeded will continue to do so.

This change is logically part of CL 401835, but since it may expose
fragile type-assertions in callers I want to keep it separate for
clearer bisection of any new test failures.

For #50436.

Change-Id: I58de1d48fb6fd788502f13657d8d4484516271cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/437176
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go
index 8e6f709a2f13b5fe086039f686fba7353c6495fa..66441ecaddd2f918f70233f06cb96ea5f26bcb64 100644 (file)
--- a/src/os/exec/exec.go
+++ b/src/os/exec/exec.go
@@ -808,8 +808,9 @@ func (c *Cmd) StdoutPipe() (io.ReadCloser, error) {
        }
        c.Stdout = pw
        c.childIOFiles = append(c.childIOFiles, pw)
-       c.parentIOPipes = append(c.parentIOPipes, pr)
-       return pr, nil
+       rc := &closeOnce{File: pr}
+       c.parentIOPipes = append(c.parentIOPipes, rc)
+       return rc, nil
 }
 
 // StderrPipe returns a pipe that will be connected to the command's
@@ -833,8 +834,9 @@ func (c *Cmd) StderrPipe() (io.ReadCloser, error) {
        }
        c.Stderr = pw
        c.childIOFiles = append(c.childIOFiles, pw)
-       c.parentIOPipes = append(c.parentIOPipes, pr)
-       return pr, nil
+       rc := &closeOnce{File: pr}
+       c.parentIOPipes = append(c.parentIOPipes, rc)
+       return rc, nil
 }
 
 // prefixSuffixSaver is an io.Writer which retains the first N bytes