From bd4c6571ecdb4851948f974333ba0f3bb8e4e6a2 Mon Sep 17 00:00:00 2001 From: Yash Anil Date: Fri, 17 Jul 2026 09:20:38 -0700 Subject: [PATCH] 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 | 13 ++++++---- fs/march/march_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/fs/march/march.go b/fs/march/march.go index effd240bd..ca1e554bf 100644 --- a/fs/march/march.go +++ b/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() diff --git a/fs/march/march_test.go b/fs/march/march_test.go index 2a916c351..4d1ef745f 100644 --- a/fs/march/march_test.go +++ b/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