Skip to content

Commit 8cb2952

Browse files
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 parent 4c02eaf commit 8cb2952

File tree

1 file changed

+3
-53
lines changed

1 file changed

+3
-53
lines changed

src/os/exec/exec.go

+3-53
Original file line numberDiff line numberDiff line change
@@ -527,16 +527,15 @@ func (c *Cmd) StdinPipe() (io.WriteCloser, error) {
527527
c.Stdin = pr
528528
c.closeAfterStart = append(c.closeAfterStart, pr)
529529
wc := &closeOnce{File: pw}
530-
c.closeAfterWait = append(c.closeAfterWait, closerFunc(wc.safeClose))
530+
c.closeAfterWait = append(c.closeAfterWait, wc)
531531
return wc, nil
532532
}
533533

534534
type closeOnce struct {
535535
*os.File
536536

537-
writers sync.RWMutex // coordinate safeClose and Write
538-
once sync.Once
539-
err error
537+
once sync.Once
538+
err error
540539
}
541540

542541
func (c *closeOnce) Close() error {
@@ -548,55 +547,6 @@ func (c *closeOnce) close() {
548547
c.err = c.File.Close()
549548
}
550549

551-
type closerFunc func() error
552-
553-
func (f closerFunc) Close() error { return f() }
554-
555-
// safeClose closes c being careful not to race with any calls to c.Write.
556-
// See golang.org/issue/9307 and TestEchoFileRace in exec_test.go.
557-
// In theory other calls could also be excluded (by writing appropriate
558-
// wrappers like c.Write's implementation below), but since c is most
559-
// commonly used as a WriteCloser, Write is the main one to worry about.
560-
// See also #7970, for which this is a partial fix for this specific instance.
561-
// The idea is that we return a WriteCloser, and so the caller can be
562-
// relied upon not to call Write and Close simultaneously, but it's less
563-
// obvious that cmd.Wait calls Close and that the caller must not call
564-
// Write and cmd.Wait simultaneously. In fact that seems too onerous.
565-
// So we change the use of Close in cmd.Wait to use safeClose, which will
566-
// synchronize with any Write.
567-
//
568-
// It's important that we know this won't block forever waiting for the
569-
// operations being excluded. At the point where this is called,
570-
// the invoked command has exited and the parent copy of the read side
571-
// of the pipe has also been closed, so there should really be no read side
572-
// of the pipe left. Any active writes should return very shortly with an EPIPE,
573-
// making it reasonable to wait for them.
574-
// Technically it is possible that the child forked a sub-process or otherwise
575-
// handed off the read side of the pipe before exiting and the current holder
576-
// is not reading from the pipe, and the pipe is full, in which case the close here
577-
// might block waiting for the write to complete. That's probably OK.
578-
// It's a small enough problem to be outweighed by eliminating the race here.
579-
func (c *closeOnce) safeClose() error {
580-
c.writers.Lock()
581-
err := c.Close()
582-
c.writers.Unlock()
583-
return err
584-
}
585-
586-
func (c *closeOnce) Write(b []byte) (int, error) {
587-
c.writers.RLock()
588-
n, err := c.File.Write(b)
589-
c.writers.RUnlock()
590-
return n, err
591-
}
592-
593-
func (c *closeOnce) WriteString(s string) (int, error) {
594-
c.writers.RLock()
595-
n, err := c.File.WriteString(s)
596-
c.writers.RUnlock()
597-
return n, err
598-
}
599-
600550
// StdoutPipe returns a pipe that will be connected to the command's
601551
// standard output when the command starts.
602552
//

0 commit comments

Comments
 (0)