Simplify shimProcess accept and wait
That we pull a value out of a channel and put it back is pretty weird.
Also we don't need a select in accept(). It's enough to just close the
listener when we learn the child is gone. (That will cancel the
Accept call.)
Change-Id: If520d9f405fa0b1ad6e3cd23e9ba8a35ff39ba75
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60887
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index e7ee194..954f2a2 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1248,8 +1248,11 @@
)
type shimProcess struct {
- cmd *exec.Cmd
- waitChan chan error
+ cmd *exec.Cmd
+ // done is closed when the process has exited. At that point, childErr may be
+ // read for the result.
+ done chan struct{}
+ childErr error
listener *net.TCPListener
stdout, stderr bytes.Buffer
}
@@ -1297,37 +1300,22 @@
return nil, err
}
- shim.waitChan = make(chan error, 1)
- go func() { shim.waitChan <- shim.cmd.Wait() }()
+ shim.done = make(chan struct{})
+ go func() {
+ shim.childErr = shim.cmd.Wait()
+ shim.listener.Close()
+ close(shim.done)
+ }()
return shim, nil
}
// accept returns a new TCP connection with the shim process, or returns an
// error on timeout or shim exit.
func (s *shimProcess) accept() (net.Conn, error) {
- type connOrError struct {
- conn net.Conn
- err error
+ if !useDebugger() {
+ s.listener.SetDeadline(time.Now().Add(*idleTimeout))
}
- connChan := make(chan connOrError, 1)
- go func() {
- if !useDebugger() {
- s.listener.SetDeadline(time.Now().Add(*idleTimeout))
- }
- conn, err := s.listener.Accept()
- connChan <- connOrError{conn, err}
- close(connChan)
- }()
- select {
- case result := <-connChan:
- return result.conn, result.err
- case childErr := <-s.waitChan:
- s.waitChan <- childErr
- if childErr == nil {
- return nil, fmt.Errorf("child exited early with no error")
- }
- return nil, fmt.Errorf("child exited early: %s", childErr)
- }
+ return s.listener.Accept()
}
// wait finishes the test and waits for the shim process to exit.
@@ -1343,9 +1331,8 @@
defer waitTimeout.Stop()
}
- err := <-s.waitChan
- s.waitChan <- err
- return err
+ <-s.done
+ return s.childErr
}
// close releases resources associated with the shimProcess. This is safe to
