[dev.fuzz] internal/fuzz: handle SIGINT races gracefully

A worker process may be terminated by SIGINT if it doesn't install the
signal handler before SIGINT is delivered. That's likely when TestMain
or the fuzz target setup take a long time. The coordinator now ignores
these errors.

Also, when testdeps.TestDeps.CoordinateFuzzing and RunFuzzWorker
return, they will send a value on the chan passed to signal.Notify
instead of closing it. This should have been obvious in hindsight, but
the signal handler could still send a value on that channel after
those functions return but before the process exits.

Change-Id: Iea2589115f1f9bb7415bb5e7911defee423e642e
Reviewed-on: https://go-review.googlesource.com/c/go/+/284292
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>

src/internal/fuzz/sys_posix.go

@@ -73,3 +73,14 @@ func getWorkerComm() (comm workerComm, err error) {
 	}
 	return workerComm{fuzzIn: fuzzIn, fuzzOut: fuzzOut, mem: mem}, nil
 }
+
+// isInterruptError returns whether an error was returned by a process that
+// was terminated by an interrupt signal (SIGINT).
+func isInterruptError(err error) bool {
+	exitErr, ok := err.(*exec.ExitError)
+	if !ok || exitErr.ExitCode() >= 0 {
+		return false
+	}
+	status := exitErr.Sys().(syscall.WaitStatus)
+	return status.Signal() == syscall.SIGINT
+}

src/internal/fuzz/sys_unimplemented.go

@@ -29,3 +29,7 @@ func setWorkerComm(cmd *exec.Cmd, comm workerComm) {
 func getWorkerComm() (comm workerComm, err error) {
 	panic("not implemented")
 }
+
+func isInterruptError(err error) bool {
+	panic("not implemented")
+}

src/internal/fuzz/sys_windows.go

@@ -131,3 +131,8 @@ func getWorkerComm() (comm workerComm, err error) {
 
 	return workerComm{fuzzIn: fuzzIn, fuzzOut: fuzzOut, mem: mem}, nil
 }
+
+func isInterruptError(err error) bool {
+	// TODO(jayconrod): implement
+	return false
+}

src/internal/fuzz/worker.go

@@ -80,19 +80,32 @@ func (w *worker) runFuzzing() error {
 		select {
 		case <-w.coordinator.doneC:
 			// All workers were told to stop.
-			return w.stop()
+			err := w.stop()
+			if isInterruptError(err) {
+				// Worker interrupted by SIGINT. This can happen if the worker receives
+				// SIGINT before installing the signal handler. That's likely if
+				// TestMain or the fuzz target setup takes a long time.
+				return nil
+			}
+			return err
 
 		case <-w.termC:
-			// Worker process terminated unexpectedly, so inform the coordinator
+			// Worker process terminated unexpectedly.
-			// that a crash occurred.
+			if isInterruptError(w.waitErr) {
+				// Worker interrupted by SIGINT. See comment in doneC case.
+				w.stop()
+				return nil
+			}
+
+			// Unexpected termination. Inform the coordinator about the crash.
+			// TODO(jayconrod,katiehockman): if -keepfuzzing, restart worker.
 			value := w.mem.valueCopy()
+			message := fmt.Sprintf("fuzzing process terminated unexpectedly: %v", w.waitErr)
 			crasher := crasherEntry{
 				corpusEntry: corpusEntry{b: value},
-				errMsg:      "fuzzing process crashed unexpectedly",
+				errMsg:      message,
 			}
 			w.coordinator.crasherC <- crasher
-
-			// TODO(jayconrod,katiehockman): if -keepfuzzing, restart worker.
 			err := w.stop()
 			if err == nil {
 				err = fmt.Errorf("worker exited unexpectedly")

src/testing/internal/testdeps/deps.go

@@ -146,7 +146,7 @@ func (TestDeps) CoordinateFuzzing(timeout time.Duration, parallel int, seed [][]
 		<-interruptC
 		cancel()
 	}()
-	defer close(interruptC)
+	defer func() { interruptC <- os.Interrupt }()
 
 	err := fuzz.CoordinateFuzzing(ctx, parallel, seed, corpusDir, cacheDir)
 	if err == ctx.Err() {
@@ -169,7 +169,7 @@ func (TestDeps) RunFuzzWorker(fn func([]byte) error) error {
 		<-interruptC
 		cancel()
 	}()
-	defer close(interruptC)
+	defer func() { interruptC <- os.Interrupt }()
 
 	err := fuzz.RunFuzzWorker(ctx, fn)
 	if err == ctx.Err() {