From c91c4cbbffdf34c62fc91d8bfda7c3d315d415e9 Mon Sep 17 00:00:00 2001
From: Sanjays2402 <51058514+Sanjays2402@users.noreply.github.com>
Date: Wed, 1 Jul 2026 17:45:11 -0700
Subject: [PATCH] accounting: fix goroutine leak in NewStatsGroup for
 zero-transfer rc jobs

NewStatsGroup started the averageLoop goroutine unconditionally at
group creation. In an rcd daemon driven by many short rc sync/move
calls (a common pattern for scheduled spool flushes), each call gets
a fresh job/N stats group. When such a job transferred zero files
the loop was never stopped, because _stopAverageLoop is only reached
via DoneTransferring once transferring and checking both go from
non-empty back to empty, which never happens if nothing was ever
transferring in the first place. The result was one leaked goroutine
per rc call, growing unbounded until the daemon was OOM-killed
(reported: ~61k goroutines and ~640 MB RSS after ~7 days from a
per-minute timer over 6 mappings).

This is the same class of leak as #8571, which fixed the equivalent
auto-start in NewStats. Fix it the same way: do not start the average
loop at group creation. NewTransfer and NewTransferRemoteSize already
call startAverageLoop when real transfer activity begins, and
DoneTransferring already stops it when the last transfer completes,
so on-demand behaviour is unchanged for groups that actually do work.
Groups that never transfer anything now cost zero goroutines.

Adds a regression test that fails without the fix.

Fixes #9567
---
 fs/accounting/stats_groups.go      |  9 ++++++-
 fs/accounting/stats_groups_test.go | 39 ++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/fs/accounting/stats_groups.go b/fs/accounting/stats_groups.go
index f14d6fed4..503ddcefa 100644
--- a/fs/accounting/stats_groups.go
+++ b/fs/accounting/stats_groups.go
@@ -301,9 +301,16 @@ func GlobalStats() *StatsInfo {
 }
 
 // NewStatsGroup creates new stats under named group.
+//
+// The averageLoop is intentionally NOT started here: it is started on
+// demand by NewTransfer / NewTransferRemoteSize when real transfer
+// activity begins and stopped by DoneTransferring when the last
+// transfer completes. Starting it eagerly leaked one goroutine per
+// group that never performed a transfer (issue #9567) - e.g. rcd
+// daemons driven by frequent rc sync/move calls where each call gets
+// a fresh job/N group and the source is often empty.
 func NewStatsGroup(ctx context.Context, group string) *StatsInfo {
 	stats := NewStats(ctx)
-	stats.startAverageLoop()
 	stats.group = group
 	groups.set(ctx, group, stats)
 	return stats
diff --git a/fs/accounting/stats_groups_test.go b/fs/accounting/stats_groups_test.go
index e0d156672..4adfb04c4 100644
--- a/fs/accounting/stats_groups_test.go
+++ b/fs/accounting/stats_groups_test.go
@@ -271,3 +271,42 @@ func percentDiff(start, end uint64) uint64 {
 	}
 	return (diff * 100) / start
 }
+
+// Regression for #9567: NewStatsGroup used to start the averageLoop
+// goroutine unconditionally at group creation. When an rcd daemon was
+// driven by many rc sync/move calls that transferred zero files, each
+// call created a fresh job/N group whose averageLoop was started but
+// never stopped (nothing ever went transferring->empty), leaking one
+// goroutine per call. The loop must only start on demand from
+// NewTransfer / NewTransferRemoteSize, and it must still stop when the
+// last transfer completes.
+func TestNewStatsGroupDoesNotStartAverageLoop(t *testing.T) {
+	ctx := context.Background()
+	defer func() {
+		groups = newStatsGroups()
+	}()
+
+	stats := NewStatsGroup(ctx, "test-group-9567")
+
+	stats.mu.RLock()
+	started := stats.average.started
+	stats.mu.RUnlock()
+	assert.False(t, started,
+		"NewStatsGroup must not start averageLoop for a zero-transfer group (issue #9567)")
+
+	// The loop must still start on demand when a real transfer is added,
+	// and stop cleanly once the last transfer completes.
+	tr := stats.NewTransferRemoteSize("regression-9567", 0, nil, nil)
+	stats.mu.RLock()
+	startedAfterTransfer := stats.average.started
+	stats.mu.RUnlock()
+	assert.True(t, startedAfterTransfer,
+		"averageLoop should start on demand when a transfer is added")
+
+	tr.Done(ctx, nil)
+	stats.mu.RLock()
+	startedAfterDone := stats.average.started
+	stats.mu.RUnlock()
+	assert.False(t, startedAfterDone,
+		"averageLoop should stop when the last transfer is done")
+}