From e042d9089f18996b13948d310b8904d039a4e42d Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Fri, 3 Mar 2023 14:17:02 +0000
Subject: [PATCH] fs: Fix interaction between --progress and --interactive

Before this change if both --progress and --interactive were set then
the screen display could become muddled.

This change makes --progress and --interactive use the same lock so
while rclone is asking for interactive questions, the progress will be
paused.

Fixes #6755
---
 cmd/progress.go             |  7 +++----
 fs/config/ui.go             |  3 +++
 fs/operations/operations.go | 24 ++++++++++++++++--------
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/cmd/progress.go b/cmd/progress.go
index 56556db2e..5468488d4 100644
--- a/cmd/progress.go
+++ b/cmd/progress.go
@@ -75,14 +75,13 @@ func startProgress() func() {
 
 // state for the progress printing
 var (
-	nlines     = 0 // number of lines in the previous stats block
-	progressMu sync.Mutex
+	nlines = 0 // number of lines in the previous stats block
 )
 
 // printProgress prints the progress with an optional log
 func printProgress(logMessage string) {
-	progressMu.Lock()
-	defer progressMu.Unlock()
+	operations.StdoutMutex.Lock()
+	defer operations.StdoutMutex.Unlock()
 
 	var buf bytes.Buffer
 	w, _ := terminal.GetSize()
diff --git a/fs/config/ui.go b/fs/config/ui.go
index 60b5b3bc6..1531884f6 100644
--- a/fs/config/ui.go
+++ b/fs/config/ui.go
@@ -46,6 +46,9 @@ func ReadNonEmptyLine(prompt string) string {
 
 // CommandDefault - choose one.  If return is pressed then it will
 // chose the defaultIndex if it is >= 0
+//
+// Must not call fs.Log anything from here to avoid deadlock in
+// --interactive --progress
 func CommandDefault(commands []string, defaultIndex int) byte {
 	opts := []string{}
 	for i, text := range commands {
diff --git a/fs/operations/operations.go b/fs/operations/operations.go
index 75f97a17f..55a24a2d0 100644
--- a/fs/operations/operations.go
+++ b/fs/operations/operations.go
@@ -836,8 +836,8 @@ func ListFn(ctx context.Context, f fs.Fs, fn func(fs.Object)) error {
 	})
 }
 
-// mutex for synchronized output
-var outMutex sync.Mutex
+// StdoutMutex mutex for synchronized output on stdout
+var StdoutMutex sync.Mutex
 
 // SyncPrintf is a global var holding the Printf function used in syncFprintf so that it can be overridden
 // Note, despite name, does not provide sync and should not be called directly
@@ -853,8 +853,8 @@ var SyncPrintf = func(format string, a ...interface{}) {
 // Updated to print to terminal if no writer is defined
 // This special behavior is used to allow easier replacement of the print to terminal code by progress
 func syncFprintf(w io.Writer, format string, a ...interface{}) {
-	outMutex.Lock()
-	defer outMutex.Unlock()
+	StdoutMutex.Lock()
+	defer StdoutMutex.Unlock()
 	if w == nil || w == os.Stdout {
 		SyncPrintf(format, a...)
 	} else {
@@ -2282,7 +2282,7 @@ func GetFsInfo(f fs.Fs) *FsInfo {
 }
 
 var (
-	interactiveMu sync.Mutex
+	interactiveMu sync.Mutex // protects the following variables
 	skipped       = map[string]bool{}
 )
 
@@ -2290,14 +2290,22 @@ var (
 //
 // Call with interactiveMu held
 func skipDestructiveChoose(ctx context.Context, subject interface{}, action string) (skip bool) {
-	fmt.Printf("rclone: %s \"%v\"?\n", action, subject)
-	switch i := config.CommandDefault([]string{
+	// Lock the StdoutMutex - must not call fs.Log anything
+	// otherwise it will deadlock with --interactive --progress
+	StdoutMutex.Lock()
+
+	fmt.Printf("\nrclone: %s \"%v\"?\n", action, subject)
+	i := config.CommandDefault([]string{
 		"yYes, this is OK",
 		"nNo, skip this",
 		fmt.Sprintf("sSkip all %s operations with no more questions", action),
 		fmt.Sprintf("!Do all %s operations with no more questions", action),
 		"qExit rclone now.",
-	}, 0); i {
+	}, 0)
+
+	StdoutMutex.Unlock()
+
+	switch i {
 	case 'y':
 		skip = false
 	case 'n':