TBR: v.io/jiri: fix a race condition in runtile/sequence.go
Share a lock across all writers passed to Capture regardless
of if they are the same or not. This handles cases of the form
var out bytes.Buffer
stdout := io.MultiWriter(&out, jirix.Stdout())
stderr := io.MultiWriter(&out, jirix.Stdout())
s.Capture(stdout, stderr).Run....
where the same underlying output stream is aliased via
a wrapper.
Change-Id: Idbe010b14de992a30c35793e6c562ce26f3406a8
diff --git a/runutil/.api b/runutil/.api
index 318b8e8..d636801 100644
--- a/runutil/.api
+++ b/runutil/.api
@@ -1,4 +1,8 @@
+pkg runutil, func GetOriginalError(error) error
+pkg runutil, func IsExist(error) bool
pkg runutil, func IsFNLHost() bool
+pkg runutil, func IsNotExist(error) bool
+pkg runutil, func IsPermission(error) bool
pkg runutil, func LookPath(string, map[string]string) (string, error)
pkg runutil, func NewRun(map[string]string, io.Reader, io.Writer, io.Writer, bool, bool, bool) *Run
pkg runutil, func NewSequence(map[string]string, io.Reader, io.Writer, io.Writer, bool, bool, bool) *Sequence
@@ -44,6 +48,7 @@
pkg runutil, method (*Sequence) Env(map[string]string) *Sequence
pkg runutil, method (*Sequence) Error() error
pkg runutil, method (*Sequence) FileExists(string) (bool, error)
+pkg runutil, method (*Sequence) Fprintf(io.Writer, string, ...interface{}) *Sequence
pkg runutil, method (*Sequence) IsDir(string) (bool, error)
pkg runutil, method (*Sequence) Last(string, ...string) error
pkg runutil, method (*Sequence) Lstat(string) (os.FileInfo, error)
@@ -65,6 +70,7 @@
pkg runutil, method (*Sequence) Symlink(string, string) *Sequence
pkg runutil, method (*Sequence) TempDir(string, string) (string, error)
pkg runutil, method (*Sequence) Timeout(time.Duration) *Sequence
+pkg runutil, method (*Sequence) Verbose(bool) *Sequence
pkg runutil, method (*Sequence) WriteFile(string, []byte, os.FileMode) *Sequence
pkg runutil, method (*Start) Command(string, ...string) (*exec.Cmd, error)
pkg runutil, method (*Start) CommandWithOpts(Opts, string, ...string) (*exec.Cmd, error)
diff --git a/runutil/sequence.go b/runutil/sequence.go
index 324e8dc..48350ab 100644
--- a/runutil/sequence.go
+++ b/runutil/sequence.go
@@ -24,6 +24,8 @@
// The first method to encounter an error short circuits any following
// methods and the result of that first error is returned by the
// Done method or any of the other 'terminating methods' (see below).
+// Sequence is not thread safe. It also good practice to use a new
+// instance of a Sequence in defer's.
//
// Unless directed to specific stdout and stderr io.Writers using Capture(),
// the stdout and stderr output from the command is discarded, unless an error
@@ -262,17 +264,29 @@
}
}
-type lockedWriter struct {
- sync.Mutex
- f io.Writer
+type sharedLockWriter struct {
+ mu *sync.Mutex
+ f io.Writer
}
-func (lw *lockedWriter) Write(d []byte) (int, error) {
- lw.Lock()
- defer lw.Unlock()
+func (lw *sharedLockWriter) Write(d []byte) (int, error) {
+ lw.mu.Lock()
+ defer lw.mu.Unlock()
return lw.f.Write(d)
}
+func serializedWriters(a, b io.Writer) (io.Writer, io.Writer) {
+ mu := new(sync.Mutex)
+ var ra, rb io.Writer
+ if a != nil {
+ ra = &sharedLockWriter{mu, a}
+ }
+ if b != nil {
+ rb = &sharedLockWriter{mu, b}
+ }
+ return ra, rb
+}
+
func (s *Sequence) initAndDefer() func() {
if s.stdout == nil && s.stderr == nil {
fout, err := ioutil.TempFile("", "seq")
@@ -312,13 +326,8 @@
opts.Stdin = s.stdin
}
var stdinCh, stderrCh chan error
- stdout := s.stdout
- stderr := s.stderr
+ stdout, stderr := serializedWriters(s.stdout, s.stderr)
if stdout != nil {
- if stdout == stderr {
- stdout = &lockedWriter{f: stdout}
- stderr = &lockedWriter{f: stderr}
- }
stdinCh = make(chan error)
go copy(stdout, rStdout, stdinCh)
} else {
