veyron/lib/modules: replace StdoutPipe with a custom in-memory queue-based
ReadWriteCloser in order to decouple the consumers of stdout from using the
stdout file handle directly.
This was motivated by two issues observed in tests:
(1) a subtle data race that popped up between reading from the StdoutPipe's
parent end we were using and closing that pipe in Wait (called by Shutdown). As
explained in http://golang.org/pkg/os/exec/#Cmd.StdoutPipe, Reading from the
pipe after Wait has been called is incorrect. Note that even if we arrange for
all Expect* calls to complete before we call Shutdown or Cleanup on the handler
or shell respectively, it is not enough: the Expect* calls spin up a goroutine
that reads from stdout, and under some circumstances (e.g. timeout) that
goroutine outlives the Expect* call that started it.
(2) handle.Shutdown hanging for a very long time in spite of having a 10 second
timeout on the Wait call. The reason was the readTo loop that was trying to
drain stdout before Shutdown could call Wait: This loop would never complete if
the child process didn't exit (since it would not see an EOF on stdout).
While we're at it, also factor out the code to transcribe stderr into a separate
routine; replace readTo by io.Copy (didn't see a reason to have our own
implementation of readTo); and stop using the command name as the log file name
in newLogfile() since that may not contain valid file name charactes.
Change-Id: Ibdcb2df79eb44672d94a1f804ba0e75e5de31524
8 files changed
