diff options
Diffstat (limited to 'src/os/exec/exec.go')
| -rw-r--r-- | src/os/exec/exec.go | 49 |
1 files changed, 46 insertions, 3 deletions
diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index d2c1b17e50..234b3bda5f 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -515,15 +515,16 @@ 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, wc) + c.closeAfterWait = append(c.closeAfterWait, closerFunc(wc.safeClose)) return wc, nil } type closeOnce struct { *os.File - once sync.Once - err error + writers sync.RWMutex // coordinate safeClose and Write + once sync.Once + err error } func (c *closeOnce) Close() error { @@ -535,6 +536,48 @@ 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 +} + // StdoutPipe returns a pipe that will be connected to the command's // standard output when the command starts. // |