]> Cypherpunks repositories - gostls13.git/commitdiff
os/exec: remove protection against simultaneous Wait/Write
authorIan Lance Taylor <iant@golang.org>
Fri, 22 Sep 2017 19:03:52 +0000 (12:03 -0700)
committerIan Lance Taylor <iant@golang.org>
Fri, 22 Sep 2017 21:10:07 +0000 (21:10 +0000)
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>

src/os/exec/exec.go

index 893d8ee99a8fd6a8c69bcca55a78a04c7fa90b7d..b0fe14d6fdd77170a29ddc238eec7214e654904c 100644 (file)
@@ -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.
 //