From d93b549f0502ad9f44b7eacc282c304b22d2603b Mon Sep 17 00:00:00 2001
From: Michael Matloob <matloob@golang.org>
Date: Mon, 6 Jan 2025 16:02:38 -0500
Subject: [PATCH] cmd/go/internal/cache: handle cacheprog not responding to
 close

Allow a gocacheprog to not respond to close. The intention of the code
is that after we send the close message we'd ignore errors reading from
the cacheprog's stdout. But before this change if a cacheprog
did not respond to close and we got an EOF reading from the cacheprog's
stdout we'd just ignore all pending requests. The send operation would
then block forever waiting for a response. With this change, we close
all response channels for pending responses if there's an error reading
from the cacheprog's stdout while we're closing. The receives from the
response channels would then proceed (but now have to handle a nil
value). Then the send operation would return and the (*ProgCache).Close
function can proceed.

Fixes #70848

Change-Id: I6631d317ba7aea3f25f714f31cd2aeef0f4d4e3e
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/640516
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
 src/cmd/go/internal/cache/prog.go             | 22 ++++++++++++++-
 .../script/build_cacheprog_issue70848.txt     | 27 +++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 src/cmd/go/testdata/script/build_cacheprog_issue70848.txt

diff --git a/src/cmd/go/internal/cache/prog.go b/src/cmd/go/internal/cache/prog.go
index 01ed9438e3..bfddf5e4de 100644
--- a/src/cmd/go/internal/cache/prog.go
+++ b/src/cmd/go/internal/cache/prog.go
@@ -153,6 +153,12 @@ func (c *ProgCache) readLoop(readLoopDone chan<- struct{}) {
 		res := new(cacheprog.Response)
 		if err := jd.Decode(res); err != nil {
 			if c.closing.Load() {
+				c.mu.Lock()
+				for _, ch := range c.inFlight {
+					close(ch)
+				}
+				c.inFlight = nil
+				c.mu.Unlock()
 				return // quietly
 			}
 			if err == io.EOF {
@@ -175,6 +181,8 @@ func (c *ProgCache) readLoop(readLoopDone chan<- struct{}) {
 	}
 }
 
+var errCacheprogClosed = errors.New("GOCACHEPROG program closed unexpectedly")
+
 func (c *ProgCache) send(ctx context.Context, req *cacheprog.Request) (*cacheprog.Response, error) {
 	resc := make(chan *cacheprog.Response, 1)
 	if err := c.writeToChild(req, resc); err != nil {
@@ -182,6 +190,9 @@ func (c *ProgCache) send(ctx context.Context, req *cacheprog.Request) (*cachepro
 	}
 	select {
 	case res := <-resc:
+		if res == nil {
+			return nil, errCacheprogClosed
+		}
 		if res.Err != "" {
 			return nil, errors.New(res.Err)
 		}
@@ -193,6 +204,9 @@ func (c *ProgCache) send(ctx context.Context, req *cacheprog.Request) (*cachepro
 
 func (c *ProgCache) writeToChild(req *cacheprog.Request, resc chan<- *cacheprog.Response) (err error) {
 	c.mu.Lock()
+	if c.inFlight == nil {
+		return errCacheprogClosed
+	}
 	c.nextID++
 	req.ID = c.nextID
 	c.inFlight[req.ID] = resc
@@ -201,7 +215,9 @@ func (c *ProgCache) writeToChild(req *cacheprog.Request, resc chan<- *cacheprog.
 	defer func() {
 		if err != nil {
 			c.mu.Lock()
-			delete(c.inFlight, req.ID)
+			if c.inFlight != nil {
+				delete(c.inFlight, req.ID)
+			}
 			c.mu.Unlock()
 		}
 	}()
@@ -348,6 +364,10 @@ func (c *ProgCache) Close() error {
 	// the context that kills the process.
 	if c.can[cacheprog.CmdClose] {
 		_, err = c.send(c.ctx, &cacheprog.Request{Command: cacheprog.CmdClose})
+		if errors.Is(err, errCacheprogClosed) {
+			// Allow the child to quit without responding to close.
+			err = nil
+		}
 	}
 	// Cancel the context, which will close the helper's stdin.
 	c.ctxCancel()
diff --git a/src/cmd/go/testdata/script/build_cacheprog_issue70848.txt b/src/cmd/go/testdata/script/build_cacheprog_issue70848.txt
new file mode 100644
index 0000000000..194fd47d93
--- /dev/null
+++ b/src/cmd/go/testdata/script/build_cacheprog_issue70848.txt
@@ -0,0 +1,27 @@
+[short] skip 'builds go programs'
+
+go build -o cacheprog$GOEXE cacheprog.go
+env GOCACHEPROG=$GOPATH/src/cacheprog$GOEXE
+
+# This should not deadlock
+go build simple.go
+! stderr 'cacheprog closed'
+
+-- simple.go --
+package main
+
+func main() {}
+-- cacheprog.go --
+// This is a minimal GOCACHEPROG program that doesn't respond to close.
+package main
+
+import (
+    "encoding/json"
+    "os"
+)
+
+func main() {
+    json.NewEncoder(os.Stdout).Encode(map[string][]string{"KnownCommands": {"close"}})
+    var res struct{}
+    json.NewDecoder(os.Stdin).Decode(&res)
+}
\ No newline at end of file
-- 
2.48.1