From cf3851c6c9380368373ac127cde1f4aa7159fba3 Mon Sep 17 00:00:00 2001
From: Adam Langley <agl@google.com>
Date: Tue, 23 May 2023 00:07:03 +0000
Subject: [PATCH] acvptool: clean up better.

The Close() method of the middle often wasn't getting called because
`os.Exit(0)` was used in some places. Once that's fixed, it's clear that
the queue of pending reads needed to be closed before waiting for the
reader goroutine to finish. Lastly, don't bother trying to record the
error that the reader saw: just panic the process if the modulewrapper
dies during processing.

Change-Id: Icf077cefd0ace2ef721a493f99fede6269531257
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60045
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
---
 util/fipstools/acvp/acvptool/acvp.go                |  6 +++---
 .../acvp/acvptool/subprocess/subprocess.go          | 13 +++++--------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/util/fipstools/acvp/acvptool/acvp.go b/util/fipstools/acvp/acvptool/acvp.go
index e92424b77..cbb07f63d 100644
--- a/util/fipstools/acvp/acvptool/acvp.go
+++ b/util/fipstools/acvp/acvptool/acvp.go
@@ -598,14 +598,14 @@ func main() {
 		}
 		os.Stdout.Write(regcapBytes)
 		os.Stdout.WriteString("\n")
-		os.Exit(0)
+		return
 	}
 
 	if len(*jsonInputFile) > 0 {
 		if err := processFile(*jsonInputFile, supportedAlgos, middle); err != nil {
 			log.Fatalf("failed to process input file: %s", err)
 		}
-		os.Exit(0)
+		return
 	}
 
 	var requestedAlgosFlag string
@@ -789,7 +789,7 @@ func main() {
 
 	if len(*fetchFlag) > 0 {
 		io.WriteString(fetchOutputTee, "]\n")
-		os.Exit(0)
+		return
 	}
 
 	if ok, err := getResultsWithRetry(server, url); err != nil {
diff --git a/util/fipstools/acvp/acvptool/subprocess/subprocess.go b/util/fipstools/acvp/acvptool/subprocess/subprocess.go
index 9167b4720..f1cb5fa88 100644
--- a/util/fipstools/acvp/acvptool/subprocess/subprocess.go
+++ b/util/fipstools/acvp/acvptool/subprocess/subprocess.go
@@ -48,8 +48,6 @@ type Subprocess struct {
 	pendingReads chan pendingRead
 	// readerFinished is a channel that is closed if `readerRoutine` has finished (e.g. because of a read error).
 	readerFinished chan struct{}
-	// readerError is set iff readerFinished is closed. If non-nil then it is the read error that caused `readerRoutine` to finished.
-	readerError error
 }
 
 // pendingRead represents an expected response from the modulewrapper.
@@ -153,6 +151,7 @@ func (m *Subprocess) Close() {
 	m.stdout.Close()
 	m.stdin.Close()
 	m.cmd.Wait()
+	close(m.pendingReads)
 	<-m.readerFinished
 }
 
@@ -176,7 +175,7 @@ func (m *Subprocess) flush() error {
 func (m *Subprocess) enqueueRead(pending pendingRead) error {
 	select {
 	case <-m.readerFinished:
-		return m.readerError
+		panic("attempted to enqueue request after the reader failed")
 	default:
 	}
 
@@ -266,7 +265,7 @@ func (m *Subprocess) Transact(cmd string, expectedNumResults int, args ...[]byte
 	case <-done:
 		return result, nil
 	case <-m.readerFinished:
-		return nil, m.readerError
+		panic("was still waiting for a result when the reader finished")
 	}
 }
 
@@ -284,13 +283,11 @@ func (m *Subprocess) readerRoutine() {
 
 		result, err := m.readResult(pendingRead.cmd, pendingRead.expectedNumResults)
 		if err != nil {
-			m.readerError = err
-			return
+			panic(fmt.Errorf("failed to read from subprocess: %w", err))
 		}
 
 		if err := pendingRead.callback(result); err != nil {
-			m.readerError = err
-			return
+			panic(fmt.Errorf("result from subprocess was rejected: %w", err))
 		}
 	}
 }