From b26db8e640e27575d80b4b2826311604cb3f6c63 Mon Sep 17 00:00:00 2001
From: Sam Lai <70988+slai@users.noreply.github.com>
Date: Thu, 22 Jun 2023 17:59:24 +0100
Subject: [PATCH] accounting: bwlimit signal handler should always start

The SIGUSR2 signal handler for bandwidth limits currently only starts
if rclone is started at a time when a bandwidth limit applies. This
means that if rclone starts _outside_ such a time, i.e. with no
bandwidth limits, then enters a time where bandwidth limits do apply,
it will not be possible to use SIGUSR2 to toggle it.

This fixes that by always starting the signal handler, but only
toggling the limiter if there is a bandwidth limit configured.
---
 fs/accounting/accounting_unix.go | 29 ++++++++++++++++++++---------
 fs/accounting/token_bucket.go    | 27 ++++++++++++---------------
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/fs/accounting/accounting_unix.go b/fs/accounting/accounting_unix.go
index ec79ce544..761466a55 100644
--- a/fs/accounting/accounting_unix.go
+++ b/fs/accounting/accounting_unix.go
@@ -23,15 +23,26 @@ func (tb *tokenBucket) startSignalHandler() {
 		// This runs forever, but blocks until the signal is received.
 		for {
 			<-signals
-			tb.mu.Lock()
-			tb.toggledOff = !tb.toggledOff
-			tb.curr, tb.prev = tb.prev, tb.curr
-			s := "disabled"
-			if !tb.curr._isOff() {
-				s = "enabled"
-			}
-			tb.mu.Unlock()
-			fs.Logf(nil, "Bandwidth limit %s by user", s)
+
+			func() {
+				tb.mu.Lock()
+				defer tb.mu.Unlock()
+
+				// if there's no bandwidth limit configured now, do nothing
+				if !tb.currLimit.Bandwidth.IsSet() {
+					fs.Debugf(nil, "SIGUSR2 received but no bandwidth limit configured right now, ignoring")
+					return
+				}
+
+				tb.toggledOff = !tb.toggledOff
+				tb.curr, tb.prev = tb.prev, tb.curr
+				s := "disabled"
+				if !tb.curr._isOff() {
+					s = "enabled"
+				}
+
+				fs.Logf(nil, "Bandwidth limit %s by user", s)
+			}()
 		}
 	}()
 }
diff --git a/fs/accounting/token_bucket.go b/fs/accounting/token_bucket.go
index c875c568f..29204b5eb 100644
--- a/fs/accounting/token_bucket.go
+++ b/fs/accounting/token_bucket.go
@@ -30,12 +30,11 @@ type buckets [TokenBucketSlots]*rate.Limiter
 
 // tokenBucket holds info about the rate limiters in use
 type tokenBucket struct {
-	mu          sync.RWMutex // protects the token bucket variables
-	curr        buckets
-	prev        buckets
-	toggledOff  bool
-	currLimitMu sync.Mutex // protects changes to the timeslot
-	currLimit   fs.BwTimeSlot
+	mu         sync.RWMutex // protects the token bucket variables
+	curr       buckets
+	prev       buckets
+	toggledOff bool
+	currLimit  fs.BwTimeSlot
 }
 
 // Return true if limit is disabled
@@ -106,11 +105,11 @@ func (tb *tokenBucket) StartTokenBucket(ctx context.Context) {
 	if tb.currLimit.Bandwidth.IsSet() {
 		tb.curr = newTokenBucket(tb.currLimit.Bandwidth)
 		fs.Infof(nil, "Starting bandwidth limiter at %v Byte/s", &tb.currLimit.Bandwidth)
-
-		// Start the SIGUSR2 signal handler to toggle bandwidth.
-		// This function does nothing in windows systems.
-		tb.startSignalHandler()
 	}
+
+	// Start the SIGUSR2 signal handler to toggle bandwidth.
+	// This function does nothing in windows systems.
+	tb.startSignalHandler()
 }
 
 // StartTokenTicker creates a ticker to update the bandwidth limiter every minute.
@@ -126,11 +125,9 @@ func (tb *tokenBucket) StartTokenTicker(ctx context.Context) {
 	go func() {
 		for range ticker.C {
 			limitNow := ci.BwLimit.LimitAt(time.Now())
-			tb.currLimitMu.Lock()
+			tb.mu.Lock()
 
 			if tb.currLimit.Bandwidth != limitNow.Bandwidth {
-				tb.mu.Lock()
-
 				// If bwlimit is toggled off, the change should only
 				// become active on the next toggle, which causes
 				// an exchange of tb.curr <-> tb.prev
@@ -156,9 +153,9 @@ func (tb *tokenBucket) StartTokenTicker(ctx context.Context) {
 				}
 
 				tb.currLimit = limitNow
-				tb.mu.Unlock()
 			}
-			tb.currLimitMu.Unlock()
+
+			tb.mu.Unlock()
 		}
 	}()
 }