bisync: fix data races on tests

cmd/bisync/bisync_test.go

@@ -228,6 +228,17 @@ var color = bisync.Color
 // TestMain drives the tests
 func TestMain(m *testing.M) {
 	bisync.LogTZ = time.UTC
+	ci := fs.GetConfig(context.TODO())
+	ciSave := *ci
+	defer func() {
+		*ci = ciSave
+	}()
+	// need to set context.TODO() here as we cannot pass a ctx to fs.LogLevelPrintf
+	ci.LogLevel = fs.LogLevelInfo
+	if *argDebug {
+		ci.LogLevel = fs.LogLevelDebug
+	}
+	fstest.Initialise()
 	fstest.TestMain(m)
 }
 
@@ -240,7 +251,8 @@ func TestBisyncRemoteLocal(t *testing.T) {
 	fs.Logf(nil, "remote: %v", remote)
 	require.NoError(t, err)
 	defer cleanup()
-	testBisync(t, remote, *argRemote2)
+	ctx, _ := fs.AddConfig(context.TODO())
+	testBisync(ctx, t, remote, *argRemote2)
 }
 
 // Path1 is local, Path2 is remote
@@ -252,7 +264,8 @@ func TestBisyncLocalRemote(t *testing.T) {
 	fs.Logf(nil, "remote: %v", remote)
 	require.NoError(t, err)
 	defer cleanup()
-	testBisync(t, *argRemote2, remote)
+	ctx, _ := fs.AddConfig(context.TODO())
+	testBisync(ctx, t, *argRemote2, remote)
 }
 
 // Path1 and Path2 are both different directories on remote
@@ -262,7 +275,8 @@ func TestBisyncRemoteRemote(t *testing.T) {
 	fs.Logf(nil, "remote: %v", remote)
 	require.NoError(t, err)
 	defer cleanup()
-	testBisync(t, remote, remote)
+	ctx, _ := fs.AddConfig(context.TODO())
+	testBisync(ctx, t, remote, remote)
 }
 
 // make sure rc can cope with running concurrent jobs
@@ -285,10 +299,7 @@ func testParallel(t *testing.T) {
 }
 
 // TestBisync is a test engine for bisync test cases.
-func testBisync(t *testing.T, path1, path2 string) {
+func testBisync(ctx context.Context, t *testing.T, path1, path2 string) {
-	ctx := context.Background()
-	fstest.Initialise()
-
 	ci := fs.GetConfig(ctx)
 	ciSave := *ci
 	defer func() {
@@ -297,7 +308,9 @@ func testBisync(t *testing.T, path1, path2 string) {
 	if *argRefreshTimes {
 		ci.RefreshTimes = true
 	}
+	bisync.ColorsLock.Lock()
 	bisync.Colors = true
+	bisync.ColorsLock.Unlock()
 	ci.FsCacheExpireDuration = fs.Duration(5 * time.Hour)
 
 	baseDir, err := os.Getwd()
@@ -618,13 +631,8 @@ func (b *bisyncTest) makeTempRemote(ctx context.Context, remote, subdir string)
 }
 
 func (b *bisyncTest) cleanupCase(ctx context.Context) {
-	// Silence "directory not found" errors from the ftp backend
+	_ = operations.Purge(ctx, b.fs1, "")
-	_ = bilib.CaptureOutput(func() {
+	_ = operations.Purge(ctx, b.fs2, "")
-		_ = operations.Purge(ctx, b.fs1, "")
-	})
-	_ = bilib.CaptureOutput(func() {
-		_ = operations.Purge(ctx, b.fs2, "")
-	})
 	_ = os.RemoveAll(b.workDir)
 	accounting.Stats(ctx).ResetCounters()
 }
@@ -639,11 +647,6 @@ func (b *bisyncTest) runTestStep(ctx context.Context, line string) (err error) {
 	defer func() {
 		*ci = ciSave
 	}()
-	ci.LogLevel = fs.LogLevelInfo
-	if b.debug {
-		ci.LogLevel = fs.LogLevelDebug
-	}
-
 	testFunc := func() {
 		src := filepath.Join(b.dataDir, "file7.txt")
 

cmd/bisync/deltas.go

@@ -396,7 +396,7 @@ func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (result
 					b.march.ls1.getPut(file, skippedDirs1)
 					b.march.ls2.getPut(file, skippedDirs2)
 					b.debugFn(file, func() {
-						b.debug(file, fmt.Sprintf("deltas dir: %s, ls1 has name?: %v, b.march.ls2 has name?: %v", file, b.march.ls1.has(b.DebugName), b.march.ls2.has(b.DebugName)))
+						b.debug(file, fmt.Sprintf("deltas dir: %s, ls1 has name?: %v, ls2 has name?: %v", file, b.march.ls1.has(b.DebugName), b.march.ls2.has(b.DebugName)))
 					})
 				} else {
 					equal := matches.Has(file)

cmd/bisync/log.go

@@ -6,6 +6,7 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/lib/encoder"
@@ -67,10 +68,15 @@ func quotePath(path string) string {
 }
 
 // Colors controls whether terminal colors are enabled
-var Colors bool
+var (
+	Colors     bool
+	ColorsLock sync.Mutex
+)
 
 // Color handles terminal colors for bisync
 func Color(style string, s string) string {
+	ColorsLock.Lock()
+	defer ColorsLock.Unlock()
 	if !Colors {
 		return s
 	}
@@ -80,6 +86,8 @@ func Color(style string, s string) string {
 
 // ColorX handles terminal colors for bisync
 func ColorX(style string, s string) string {
+	ColorsLock.Lock()
+	defer ColorsLock.Unlock()
 	if !Colors {
 		return s
 	}

cmd/bisync/operations.go

@@ -87,7 +87,9 @@ func Bisync(ctx context.Context, fs1, fs2 fs.Fs, optArg *Options) (err error) {
 	opt.OrigBackupDir = ci.BackupDir
 
 	if ci.TerminalColorMode == fs.TerminalColorModeAlways || (ci.TerminalColorMode == fs.TerminalColorModeAuto && !log.Redirected()) {
+		ColorsLock.Lock()
 		Colors = true
+		ColorsLock.Unlock()
 	}
 
 	err = b.setCompareDefaults(ctx)

cmd/bisync/queue.go

@@ -245,8 +245,10 @@ func (b *bisyncRun) fastCopy(ctx context.Context, fsrc, fdst fs.Fs, files bilib.
 		}
 	}
 
-	b.SyncCI = fs.GetConfig(ctxCopy)      // allows us to request graceful shutdown
+	b.SyncCI = fs.GetConfig(ctxCopy) // allows us to request graceful shutdown
-	accounting.MaxCompletedTransfers = -1 // we need a complete list in the event of graceful shutdown
+	if accounting.MaxCompletedTransfers != -1 {
+		accounting.MaxCompletedTransfers = -1 // we need a complete list in the event of graceful shutdown
+	}
 	ctxCopy, b.CancelSync = context.WithCancel(ctxCopy)
 	b.testFn()
 	err := sync.Sync(ctxCopy, fdst, fsrc, b.opt.CreateEmptySrcDirs)

cmd/bisync/resync.go

@@ -20,7 +20,6 @@ func (b *bisyncRun) setResyncDefaults() {
 	}
 	if b.opt.ResyncMode != PreferNone {
 		b.opt.Resync = true
-		Opt.Resync = true // shouldn't be using this one, but set to be safe
 	}
 
 	// checks and warnings

fs/log/slog.go

@@ -29,7 +29,7 @@ var Handler = defaultHandler()
 // InitLogging has been called yet or not.
 func defaultHandler() *OutputHandler {
 	// Default options for default handler
-	var opts = &slog.HandlerOptions{
+	opts := &slog.HandlerOptions{
 		Level: fs.LogLevelToSlog(fs.InitialLogLevel()),
 	}
 
@@ -87,7 +87,7 @@ func getCaller(skip int) string {
 		return ""
 	}
 	frames := runtime.CallersFrames(pc[:n])
-	var more = true
+	more := true
 	var frame runtime.Frame
 	for more {
 		frame, more = frames.Next()
@@ -175,11 +175,15 @@ func NewOutputHandler(out io.Writer, opts *slog.HandlerOptions, format logFormat
 //
 // This is for temporarily overriding the output.
 func (h *OutputHandler) SetOutput(fn outputFn) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.output = append(h.output, fn)
 }
 
 // ResetOutput resets the log output to what is was.
 func (h *OutputHandler) ResetOutput() {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	if len(h.output) > 0 {
 		h.output = h.output[:len(h.output)-1]
 	}
@@ -187,6 +191,8 @@ func (h *OutputHandler) ResetOutput() {
 
 // AddOutput adds an additional logging destination of the type specified.
 func (h *OutputHandler) AddOutput(json bool, fn outputFn) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.outputExtra = append(h.outputExtra, outputExtra{
 		json:   json,
 		output: fn,
@@ -195,6 +201,8 @@ func (h *OutputHandler) AddOutput(json bool, fn outputFn) {
 
 // SetLevel sets a new log level, returning the old one.
 func (h *OutputHandler) SetLevel(level slog.Level) slog.Level {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	oldLevel := h.levelVar.Level()
 	h.levelVar.Set(level)
 	return oldLevel