os/exec: remove protection against simultaneous Wait/Write

CL 31148 added code to protect again simultaneous calls to Close and
Wait when using the standard input pipe, to fix the race condition
described in issue #9307. That issue is a special case of the race
between Close and Write described by issue #7970. Since issue #7970
was not fixed, CL 31148 fixed the problem specific to os/exec.

Since then, issue #7970 has been fixed, so the specific fix in os/exec
is no longer necessary. Remove it, effectively reverting CL 31148 and
followup CL 33298.

Updates #7970
Updates #9307
Updates #17647

Change-Id: Ic0b62569cb0aba44b32153cf5f9632bd1f1b411a
Reviewed-on: https://go-review.googlesource.com/65490
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Miguel Bernabeu <miguelbernadi@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

1 files changed, 3 insertions, 53 deletions

diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go
index 893d8ee99a..b0fe14d6fd 100644
--- a/src/os/exec/exec.go
+++ b/src/os/exec/exec.go
@@ -527,16 +527,15 @@ func (c *Cmd) StdinPipe() (io.WriteCloser, error) {
 	c.Stdin = pr
 	c.closeAfterStart = append(c.closeAfterStart, pr)
 	wc := &closeOnce{File: pw}
-	c.closeAfterWait = append(c.closeAfterWait, closerFunc(wc.safeClose))
+	c.closeAfterWait = append(c.closeAfterWait, wc)
 	return wc, nil
 }
 
 type closeOnce struct {
 	*os.File
 
-	writers sync.RWMutex // coordinate safeClose and Write
-	once    sync.Once
-	err     error
+	once sync.Once
+	err  error
 }
 
 func (c *closeOnce) Close() error {
@@ -548,55 +547,6 @@ func (c *closeOnce) close() {
 	c.err = c.File.Close()
 }
 
-type closerFunc func() error
-
-func (f closerFunc) Close() error { return f() }
-
-// safeClose closes c being careful not to race with any calls to c.Write.
-// See golang.org/issue/9307 and TestEchoFileRace in exec_test.go.
-// In theory other calls could also be excluded (by writing appropriate
-// wrappers like c.Write's implementation below), but since c is most
-// commonly used as a WriteCloser, Write is the main one to worry about.
-// See also #7970, for which this is a partial fix for this specific instance.
-// The idea is that we return a WriteCloser, and so the caller can be
-// relied upon not to call Write and Close simultaneously, but it's less
-// obvious that cmd.Wait calls Close and that the caller must not call
-// Write and cmd.Wait simultaneously. In fact that seems too onerous.
-// So we change the use of Close in cmd.Wait to use safeClose, which will
-// synchronize with any Write.
-//
-// It's important that we know this won't block forever waiting for the
-// operations being excluded. At the point where this is called,
-// the invoked command has exited and the parent copy of the read side
-// of the pipe has also been closed, so there should really be no read side
-// of the pipe left. Any active writes should return very shortly with an EPIPE,
-// making it reasonable to wait for them.
-// Technically it is possible that the child forked a sub-process or otherwise
-// handed off the read side of the pipe before exiting and the current holder
-// is not reading from the pipe, and the pipe is full, in which case the close here
-// might block waiting for the write to complete. That's probably OK.
-// It's a small enough problem to be outweighed by eliminating the race here.
-func (c *closeOnce) safeClose() error {
-	c.writers.Lock()
-	err := c.Close()
-	c.writers.Unlock()
-	return err
-}
-
-func (c *closeOnce) Write(b []byte) (int, error) {
-	c.writers.RLock()
-	n, err := c.File.Write(b)
-	c.writers.RUnlock()
-	return n, err
-}
-
-func (c *closeOnce) WriteString(s string) (int, error) {
-	c.writers.RLock()
-	n, err := c.File.WriteString(s)
-	c.writers.RUnlock()
-	return n, err
-}
-
 // StdoutPipe returns a pipe that will be connected to the command's
 // standard output when the command starts.
 //