From 9d3b8d9a9f1c69b9ae1947708aa907250c7ac507 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sat, 1 Jul 2023 15:28:10 +0100 Subject: [PATCH] operations: fix deadlock when using lsd/ls with --progress - Fixes #7102 The --progress flag overrides operations.SyncPrintf in order to do its magic on stdout without interfering with other output. Before this change the syncFprintf routine in operations (which is used to print all output to stdout) was taking the operations.StdoutMutex and the printProgress function in the --progress routine was also attempting to take the same mutex causing a deadlock. This patch fixes the problem by moving the locking from the syncFprintf function to SyncPrintf. It is then up to the function overriding this to lock the StdoutMutex. This ensures the StdoutMutex can never cause a deadlock. --- fs/operations/operations.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 8c20fbe40..3c89bbc97 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -926,10 +926,15 @@ func ListFn(ctx context.Context, f fs.Fs, fn func(fs.Object)) error { // 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 -// Call syncFprintf, which provides sync +// SyncPrintf is a global var holding the Printf function so that it +// can be overridden. +// +// This writes to stdout holding the StdoutMutex. If you are going to +// override it and write to os.Stdout then you should hold the +// StdoutMutex too. var SyncPrintf = func(format string, a ...interface{}) { + StdoutMutex.Lock() + defer StdoutMutex.Unlock() fmt.Printf(format, a...) } @@ -937,14 +942,13 @@ var SyncPrintf = func(format string, a ...interface{}) { // // Ignores errors from Fprintf. // -// 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 +// Prints to stdout if w is nil func syncFprintf(w io.Writer, format string, a ...interface{}) { - StdoutMutex.Lock() - defer StdoutMutex.Unlock() if w == nil || w == os.Stdout { SyncPrintf(format, a...) } else { + StdoutMutex.Lock() + defer StdoutMutex.Unlock() _, _ = fmt.Fprintf(w, format, a...) } }