Stop FIFO reader after terminal errors

client_notification_test.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"io"
+	"strings"
 	"testing"
 
 	"git.stormux.org/storm/barnard/gumble/gumble"
@@ -15,6 +17,23 @@ func TestEscRemovesTerminalControlSequences(t *testing.T) {
 	}
 }
 
+type testReadCloser struct{ io.Reader }
+
+func (testReadCloser) Close() error { return nil }
+
+// Regression: an EOF from the FIFO was ignored and caused an unbounded busy
+// loop. The reader must deliver a final command then close its output.
+func TestReadFIFOStopsOnEOF(t *testing.T) {
+	out := make(chan string)
+	go readFIFO(testReadCloser{strings.NewReader("command\n")}, out)
+	if got := <-out; got != "command" {
+		t.Fatalf("got %q", got)
+	}
+	if _, ok := <-out; ok {
+		t.Fatal("FIFO output remained open after EOF")
+	}
+}
+
 func TestUserChangeNotification(t *testing.T) {
 	current := &gumble.Channel{ID: 1, Name: "Current"}
 	other := &gumble.Channel{ID: 2, Name: "Other"}

fix.txt

@@ -225,7 +225,7 @@ Priority 3: configuration, UI, and binding hardening
     temporary file/directory before rename. Avoid broad unrelated formatting
     changes while fixing this.
 
-29. FIFO reader spins after an error
+[x] 29. FIFO reader spins after an error
     File: main.go
     setup_fifo ignores all ReadBytes errors and retries immediately. It also
     never closes the FIFO descriptor. Exit the reader on terminal errors,

main.go

@@ -88,18 +88,27 @@ func setup_fifo(fn string) (chan string, error) {
 	if err != nil {
 		return t, err
 	}
-	go func(fh io.Reader, out chan string) {
+	go readFIFO(file, t)
-		reader := bufio.NewReader(fh)
-		for {
-			line, err := reader.ReadBytes('\n')
-			if err == nil {
-				out <- strings.TrimSpace(string(line))
-			}
-		}
-	}(file, t)
 	return t, nil
 }
 
+// readFIFO forwards complete commands and terminates on EOF or any read
+// failure. Retrying an unrecoverable FIFO error used to spin a CPU forever.
+func readFIFO(fh io.ReadCloser, out chan<- string) {
+	defer fh.Close()
+	defer close(out)
+	reader := bufio.NewReader(fh)
+	for {
+		line, err := reader.ReadBytes('\n')
+		if len(line) != 0 {
+			out <- strings.TrimSpace(string(line))
+		}
+		if err != nil {
+			return
+		}
+	}
+}
+
 func main() {
 	// Command line flags
 	server := flag.String("server", "localhost:64738", "the server to connect to")