internal/fuzz: release lock when reading file fails

When corpusEntryData failed in workerClient.fuzz and
workerClient.minimize, the shared memory mutex wasn't properly given up,
which would cause a deadlock when worker.cleanup was called.

This was tickled by #59062, wherein the fuzz cache directory would be
removed during operation of the fuzzer, causing corpusEntryData to fail
because the entry files no longer existed.

Updates #51484

Change-Id: Iea284041c20d1581c662bddbbc7e12191771a364
Reviewed-on: https://go-review.googlesource.com/c/go/+/476815
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>

diff --git a/src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt b/src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt
new file mode 100644 (file)
index 0000000..4feb41a
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt
@@ -0,0 +1,50 @@
+[short] skip
+[!fuzz-instrumented] skip
+
+env GOCACHE=$WORK/cache
+! go test -fuzz=FuzzDead -v
+# This is a somewhat inexact check, but since we don't prefix the error with anything
+# and as the error suffix is platform dependent, this is the best we can do. In the
+# deadlock failure case, the test will just deadlock and timeout anyway, so it should
+# be clear that that failure mode is different.
+stdout 'open'
+
+-- go.mod --
+module test
+
+-- cov_test.go --
+package dead
+
+import (
+       "os"
+       "path/filepath"
+       "testing"
+       "time"
+)
+
+func FuzzDead(f *testing.F) {
+       go func() {
+               c := filepath.Join(os.Getenv("GOCACHE"), "fuzz", "test", "FuzzDead")
+               t := time.NewTicker(time.Second)
+               for range t.C {
+                       files, _ := os.ReadDir(c)
+                       if len(files) > 0 {
+                               os.RemoveAll(c)
+                       }
+               }
+       }()
+
+       f.Fuzz(func(t *testing.T, b []byte) {
+               if len(b) == 8 &&
+                       b[0] == 'h' &&
+                       b[1] == 'e' &&
+                       b[2] == 'l' &&
+                       b[3] == 'l' &&
+                       b[4] == 'o' &&
+                       b[5] == ' ' &&
+                       b[6] == ':' &&
+                       b[7] == ')' {
+                       return
+               }
+       })
+}

diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go
index d40245a3f2f1e738a870bcfefb42b2451c36d28b..c9526709951fee4ab578fd719a505fbaa46c0796 100644 (file)
--- a/src/internal/fuzz/worker.go
+++ b/src/internal/fuzz/worker.go
@@ -993,13 +993,13 @@ func (wc *workerClient) minimize(ctx context.Context, entryIn CorpusEntry, args
        if !ok {
                return CorpusEntry{}, minimizeResponse{}, errSharedMemClosed
        }
+       defer func() { wc.memMu <- mem }()
        mem.header().count = 0
        inp, err := corpusEntryData(entryIn)
        if err != nil {
                return CorpusEntry{}, minimizeResponse{}, err
        }
        mem.setValue(inp)
-       defer func() { wc.memMu <- mem }()
        entryOut = entryIn
        entryOut.Values, err = unmarshalCorpusFile(inp)
        if err != nil {
@@ -1082,6 +1082,7 @@ func (wc *workerClient) fuzz(ctx context.Context, entryIn CorpusEntry, args fuzz
        mem.header().count = 0
        inp, err := corpusEntryData(entryIn)
        if err != nil {
+               wc.memMu <- mem
                return CorpusEntry{}, fuzzResponse{}, true, err
        }
        mem.setValue(inp)