Merge pull request #5300 from MichaelEischer/fix-output-race

ui/termstatus: fix race condition in StdioWrapper

changelog/unreleased/issue-5259

@@ -0,0 +1,16 @@
+Bugfix: Fix rare crash in command output
+
+Some commands could in rare cases crash when trying to print status messages
+and request retries at the same time. This resulted in an error like the following:
+
+```
+panic: runtime error: slice bounds out of range [468:156]
+[...]
+github.com/restic/restic/internal/ui/termstatus.(*lineWriter).Write(...)
+	/restic/internal/ui/termstatus/stdio_wrapper.go:36 +0x136
+```
+
+This has been fixed.
+
+https://github.com/restic/restic/issues/5259
+https://github.com/restic/restic/pull/5300

internal/ui/termstatus/stdio_wrapper.go

@@ -3,6 +3,7 @@ package termstatus
 import (
 	"bytes"
 	"io"
+	"sync"
 )
 
 // WrapStdio returns line-buffering replacements for os.Stdout and os.Stderr.
@@ -12,6 +13,7 @@ func WrapStdio(term *Terminal) (stdout, stderr io.WriteCloser) {
 }
 
 type lineWriter struct {
+	m     sync.Mutex
 	buf   bytes.Buffer
 	print func(string)
 }
@@ -23,6 +25,9 @@ func newLineWriter(print func(string)) *lineWriter {
 }
 
 func (w *lineWriter) Write(data []byte) (n int, err error) {
+	w.m.Lock()
+	defer w.m.Unlock()
+
 	n, err = w.buf.Write(data)
 	if err != nil {
 		return n, err
@@ -40,6 +45,9 @@ func (w *lineWriter) Write(data []byte) (n int, err error) {
 }
 
 func (w *lineWriter) Close() error {
+	w.m.Lock()
+	defer w.m.Unlock()
+
 	if w.buf.Len() > 0 {
 		w.print(string(append(w.buf.Bytes(), '\n')))
 	}

internal/ui/termstatus/stdio_wrapper_test.go

@@ -1,10 +1,13 @@
 package termstatus
 
 import (
+	"context"
 	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	rtest "github.com/restic/restic/internal/test"
+	"golang.org/x/sync/errgroup"
 )
 
 func TestStdioWrapper(t *testing.T) {
@@ -82,3 +85,18 @@ func TestStdioWrapper(t *testing.T) {
 		})
 	}
 }
+
+func TestStdioWrapperConcurrentWrites(t *testing.T) {
+	// tests for race conditions when run with `go test -race ./internal/ui/termstatus`
+	w := newLineWriter(func(_ string) {})
+
+	wg, _ := errgroup.WithContext(context.TODO())
+	for range 5 {
+		wg.Go(func() error {
+			_, err := w.Write([]byte("test\n"))
+			return err
+		})
+	}
+	rtest.OK(t, wg.Wait())
+	rtest.OK(t, w.Close())
+}