From 9a029a69157788d072147732fe2665c58bbbc02c Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 30 Sep 2022 11:08:54 -0400 Subject: [PATCH] os/exec: make StdoutPipe and StderrPipe safe to Close concurrently MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills TryBot-Result: Gopher Robot --- src/os/exec/exec.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index 8e6f709a2f..66441ecadd 100644 --- 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 -- 2.50.0