march: fix goroutine leak on completed async rc jobs - fixes #9620

The march janitor goroutine, which discards queued jobs when the context is
cancelled, only ever returned on context cancellation. A march that finished
normally never cancels its context, so on an async rc job (whose context
descends from context.Background and is only cancelled by job/stop) the
janitor parked forever, leaking one goroutine per run and pinning that run's
directory listings in memory. A long-running rcd driving async sync or bisync
jobs accumulated these until it ran out of memory.

Signal the janitor to exit once the march completes so it returns on both
normal completion and cancellation.

fs/march/march.go

@@ -263,14 +263,19 @@ func (m *March) Run(ctx context.Context) error {
 		dstDepth:  dstDepth - 1,
 		noDst:     m.NoCheckDest,
 	}
+	// Discard the remaining jobs on cancellation to unblock the senders.
+	done := make(chan struct{})
 	go func() {
-		// when the context is cancelled discard the remaining jobs
-		<-m.Ctx.Done()
-		for range in {
-			traversing.Done()
+		select {
+		case <-m.Ctx.Done():
+			for range in {
+				traversing.Done()
+			}
+		case <-done:
 		}
 	}()
 	traversing.Wait()
+	close(done)
 	close(in)
 	wg.Wait()
 

fs/march/march_test.go

@@ -6,9 +6,11 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"runtime"
 	"strings"
 	"sync"
 	"testing"
+	"time"
 
 	_ "github.com/rclone/rclone/backend/local"
 	"github.com/rclone/rclone/fs"
@@ -378,6 +380,58 @@ func TestMarchNoProcessDstOnly(t *testing.T) {
 	}
 }
 
+// TestMarchNoGoroutineLeak checks that a march which completes normally (its
+// context is never cancelled, as with an async rc job) does not strand the
+// janitor goroutine that drains the job channel (See #9620)
+func TestMarchNoGoroutineLeak(t *testing.T) {
+	r := fstest.NewRun(t)
+
+	ctx := context.Background()
+	ctx, _ = fs.AddConfig(ctx)
+	// Write to both src and dst so the march lists real directories on both
+	// sides and completes cleanly (no missing-directory listing errors).
+	r.WriteBoth(ctx, "test", "hello world", t1)
+	r.WriteBoth(ctx, "sub dir/test2", "hello world", t1)
+
+	runMarch := func() {
+		mt := &marchTester{ctx: ctx, cancel: func() {}}
+		m := &March{
+			Ctx:      ctx,
+			Fdst:     r.Fremote,
+			Fsrc:     r.Flocal,
+			Dir:      "",
+			Callback: mt,
+		}
+		require.NoError(t, m.Run(ctx))
+	}
+
+	// Warm up so any one-time background goroutines are already started.
+	runMarch()
+	waitForGoroutines := func() int {
+		// Give any exiting goroutines a moment to unwind before counting.
+		var n int
+		for range 20 {
+			runtime.GC()
+			n = runtime.NumGoroutine()
+			time.Sleep(10 * time.Millisecond)
+		}
+		return n
+	}
+	before := waitForGoroutines()
+
+	const runs = 20
+	for range runs {
+		runMarch()
+	}
+	after := waitForGoroutines()
+
+	// The leak stranded one goroutine per run. Allow a small slack for unrelated
+	// runtime goroutines, but well under the number of runs.
+	if after-before >= runs {
+		t.Fatalf("goroutine leak: %d before, %d after %d marches (delta %d)", before, after, runs, after-before)
+	}
+}
+
 // matchPair is a matched pair of direntries returned by matchListings
 type matchPair struct {
 	src, dst fs.DirEntry