catfile: Stop cache to plug Goroutine leaks

In many tests where the catfile cache is in use, we create it but don't
ever clean up the cache. As a result, the monitoring Goroutine and any
potenitally cached processes may not get cleaned up as expected.

Fix this by calling `Stop()` as required.

21 files changed, 75 insertions, 19 deletions

diff --git a/internal/git/gittest/commit_test.go b/internal/git/gittest/commit_test.go
index e96520a14..c3ad815ce 100644
--- a/internal/git/gittest/commit_test.go
+++ b/internal/git/gittest/commit_test.go
@@ -19,6 +19,8 @@ func TestWriteCommit(t *testing.T) {
 	defer cancel()
 
 	batchCache := catfile.NewCache(cfg)
+	defer batchCache.Stop()
+
 	batch, err := batchCache.BatchProcess(ctx, repo)
 	require.NoError(t, err)
 
diff --git a/internal/git/localrepo/config_test.go b/internal/git/localrepo/config_test.go
index 15c440524..7abc7be08 100644
--- a/internal/git/localrepo/config_test.go
+++ b/internal/git/localrepo/config_test.go
@@ -31,7 +31,10 @@ func setupRepoConfig(t *testing.T) (Config, string) {
 	repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
 
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
-	repo := New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+
+	repo := New(gitCmdFactory, catfileCache, repoProto, cfg)
 
 	return repo.Config(), repoPath
 }
diff --git a/internal/git/localrepo/objects_test.go b/internal/git/localrepo/objects_test.go
index d1d797955..53c420d6b 100644
--- a/internal/git/localrepo/objects_test.go
+++ b/internal/git/localrepo/objects_test.go
@@ -35,7 +35,9 @@ func setupRepo(t *testing.T, bare bool) (*Repo, string) {
 	}
 
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
-	return New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg), repoPath
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+	return New(gitCmdFactory, catfileCache, repoProto, cfg), repoPath
 }
 
 type ReaderFunc func([]byte) (int, error)
diff --git a/internal/git/localrepo/refs_test.go b/internal/git/localrepo/refs_test.go
index 3d3fdcd32..6bdc9a7e1 100644
--- a/internal/git/localrepo/refs_test.go
+++ b/internal/git/localrepo/refs_test.go
@@ -242,9 +242,12 @@ func TestRepo_GetRemoteReferences(t *testing.T) {
 	annotatedTagOID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "annotated-tag"))
 
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
+	catfileCache := catfile.NewCache(cfg)
+	defer catfileCache.Stop()
+
 	repo := New(
 		gitCmdFactory,
-		catfile.NewCache(cfg),
+		catfileCache,
 		&gitalypb.Repository{StorageName: "default", RelativePath: filepath.Join(relativePath, ".git")},
 		cfg,
 	)
diff --git a/internal/git/localrepo/remote_test.go b/internal/git/localrepo/remote_test.go
index 6b165f0c9..547f0cf90 100644
--- a/internal/git/localrepo/remote_test.go
+++ b/internal/git/localrepo/remote_test.go
@@ -37,7 +37,10 @@ func setupRepoRemote(t *testing.T, bare bool) (Remote, string) {
 	}
 
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
-	return New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg).Remote(), repoPath
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+
+	return New(gitCmdFactory, catfileCache, repoProto, cfg).Remote(), repoPath
 }
 
 func TestRepo_Remote(t *testing.T) {
diff --git a/internal/git/localrepo/repo.go b/internal/git/localrepo/repo.go
index 08c996b18..af869a6bb 100644
--- a/internal/git/localrepo/repo.go
+++ b/internal/git/localrepo/repo.go
@@ -37,7 +37,9 @@ func New(gitCmdFactory git.CommandFactory, catfileCache catfile.Cache, repo repo
 // dependencies ad-hoc from the given config.
 func NewTestRepo(t testing.TB, cfg config.Cfg, repo repository.GitRepo) *Repo {
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
-	return New(gitCmdFactory, catfile.NewCache(cfg), repo, cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+	return New(gitCmdFactory, catfileCache, repo, cfg)
 }
 
 // Path returns the on-disk path of the repository.
diff --git a/internal/git/localrepo/repo_test.go b/internal/git/localrepo/repo_test.go
index 8993471e0..c456e21b3 100644
--- a/internal/git/localrepo/repo_test.go
+++ b/internal/git/localrepo/repo_test.go
@@ -20,7 +20,9 @@ func TestRepo(t *testing.T) {
 	gittest.TestRepository(t, cfg, func(t testing.TB, pbRepo *gitalypb.Repository) git.Repository {
 		t.Helper()
 		gitCmdFactory := git.NewExecCommandFactory(cfg)
-		return New(gitCmdFactory, catfile.NewCache(cfg), pbRepo, cfg)
+		catfileCache := catfile.NewCache(cfg)
+		t.Cleanup(catfileCache.Stop)
+		return New(gitCmdFactory, catfileCache, pbRepo, cfg)
 	})
 }
 
@@ -28,7 +30,9 @@ func TestRepo_Path(t *testing.T) {
 	t.Run("valid repository", func(t *testing.T) {
 		cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
 		gitCmdFactory := git.NewExecCommandFactory(cfg)
-		repo := New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg)
+		catfileCache := catfile.NewCache(cfg)
+		t.Cleanup(catfileCache.Stop)
+		repo := New(gitCmdFactory, catfileCache, repoProto, cfg)
 
 		path, err := repo.Path()
 		require.NoError(t, err)
@@ -38,7 +42,9 @@ func TestRepo_Path(t *testing.T) {
 	t.Run("deleted repository", func(t *testing.T) {
 		cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
 		gitCmdFactory := git.NewExecCommandFactory(cfg)
-		repo := New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg)
+		catfileCache := catfile.NewCache(cfg)
+		t.Cleanup(catfileCache.Stop)
+		repo := New(gitCmdFactory, catfileCache, repoProto, cfg)
 
 		require.NoError(t, os.RemoveAll(repoPath))
 
@@ -49,7 +55,9 @@ func TestRepo_Path(t *testing.T) {
 	t.Run("non-git repository", func(t *testing.T) {
 		cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
 		gitCmdFactory := git.NewExecCommandFactory(cfg)
-		repo := New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg)
+		catfileCache := catfile.NewCache(cfg)
+		t.Cleanup(catfileCache.Stop)
+		repo := New(gitCmdFactory, catfileCache, repoProto, cfg)
 
 		// Recreate the repository as a simple empty directory to simulate
 		// that the repository is in a partially-created state.
diff --git a/internal/git/objectpool/clone_test.go b/internal/git/objectpool/clone_test.go
index 0720170a0..f3689ef24 100644
--- a/internal/git/objectpool/clone_test.go
+++ b/internal/git/objectpool/clone_test.go
@@ -23,11 +23,14 @@ func setupObjectPool(t *testing.T) (*ObjectPool, *gitalypb.Repository) {
 	cfg, repo, _ := testcfg.BuildWithRepo(t)
 	gitCommandFactory := git.NewExecCommandFactory(cfg)
 
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+
 	pool, err := NewObjectPool(
 		cfg,
 		config.NewLocator(cfg),
 		gitCommandFactory,
-		catfile.NewCache(cfg),
+		catfileCache,
 		transaction.NewManager(cfg, backchannel.NewRegistry()),
 		repo.GetStorageName(),
 		gittest.NewObjectPoolName(t),
diff --git a/internal/gitaly/maintenance/optimize_test.go b/internal/gitaly/maintenance/optimize_test.go
index 19d6ee5ca..0281bf843 100644
--- a/internal/gitaly/maintenance/optimize_test.go
+++ b/internal/gitaly/maintenance/optimize_test.go
@@ -33,6 +33,7 @@ func (mo *mockOptimizer) OptimizeRepository(ctx context.Context, req *gitalypb.O
 	l := config.NewLocator(mo.cfg)
 	gitCmdFactory := git.NewExecCommandFactory(mo.cfg)
 	catfileCache := catfile.NewCache(mo.cfg)
+	mo.t.Cleanup(catfileCache.Stop)
 	resp, err := repository.NewServer(mo.cfg, nil, l, transaction.NewManager(mo.cfg, backchannel.NewRegistry()), gitCmdFactory, catfileCache).OptimizeRepository(ctx, req)
 	assert.NoError(mo.t, err)
 	return resp, err
diff --git a/internal/gitaly/server/auth_test.go b/internal/gitaly/server/auth_test.go
index 43edf2c03..803c2680d 100644
--- a/internal/gitaly/server/auth_test.go
+++ b/internal/gitaly/server/auth_test.go
@@ -194,6 +194,7 @@ func runServer(t *testing.T, cfg config.Cfg) string {
 	), cfg)
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
 	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
 	diskCache := cache.New(cfg, locator)
 
 	srv, err := New(false, cfg, testhelper.DiscardTestEntry(t), registry, diskCache)
diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
index 9b68557e1..ba48dd00c 100644
--- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
+++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
@@ -162,11 +162,14 @@ func TestFetchIntoObjectPool_Failure(t *testing.T) {
 
 	locator := config.NewLocator(cfg)
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
+
 	server := NewServer(
 		cfg,
 		locator,
 		gitCmdFactory,
-		catfile.NewCache(cfg),
+		catfileCache,
 		transaction.NewManager(cfg, backchannel.NewRegistry()),
 	)
 
diff --git a/internal/gitaly/service/objectpool/testhelper_test.go b/internal/gitaly/service/objectpool/testhelper_test.go
index 2ffd56e74..86b85adcc 100644
--- a/internal/gitaly/service/objectpool/testhelper_test.go
+++ b/internal/gitaly/service/objectpool/testhelper_test.go
@@ -69,12 +69,14 @@ func initObjectPool(t testing.TB, cfg config.Cfg, storage config.Storage) *objec
 
 	relativePath := gittest.NewObjectPoolName(t)
 	gittest.InitRepoDir(t, storage.Path, relativePath)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
 
 	pool, err := objectpool.NewObjectPool(
 		cfg,
 		config.NewLocator(cfg),
 		git.NewExecCommandFactory(cfg),
-		catfile.NewCache(cfg),
+		catfileCache,
 		transaction.NewManager(cfg, backchannel.NewRegistry()),
 		storage.Name,
 		relativePath,
diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go
index bc128093c..ba6666b35 100644
--- a/internal/gitaly/service/operations/apply_patch_test.go
+++ b/internal/gitaly/service/operations/apply_patch_test.go
@@ -279,7 +279,10 @@ To restore the original branch and stop patching, run "git am --abort".
 		t.Run(tc.desc, func(t *testing.T) {
 			repoPb, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
 
-			repo := localrepo.New(git.NewExecCommandFactory(cfg), catfile.NewCache(cfg), repoPb, cfg)
+			catfileCache := catfile.NewCache(cfg)
+			t.Cleanup(catfileCache.Stop)
+
+			repo := localrepo.New(git.NewExecCommandFactory(cfg), catfileCache, repoPb, cfg)
 
 			executor := git2go.NewExecutor(cfg, config.NewLocator(cfg))
 
diff --git a/internal/gitaly/service/ref/find_all_tags_test.go b/internal/gitaly/service/ref/find_all_tags_test.go
index 628edc948..86aebd495 100644
--- a/internal/gitaly/service/ref/find_all_tags_test.go
+++ b/internal/gitaly/service/ref/find_all_tags_test.go
@@ -399,6 +399,8 @@ func TestFindAllTags_nestedTags(t *testing.T) {
 			testhelper.MustRunCommand(t, tags, "xargs", cfg.Git.BinPath, "-C", repoPath, "tag", "-d")
 
 			catfileCache := catfile.NewCache(cfg)
+			defer catfileCache.Stop()
+
 			batch, err := catfileCache.BatchProcess(ctx, repo)
 			require.NoError(t, err)
 
@@ -506,7 +508,10 @@ func TestFindAllTags_sorted(t *testing.T) {
 
 	repoProto, _ := gittest.CloneRepo(t, cfg, cfg.Storages[0])
 
-	repo := localrepo.New(git.NewExecCommandFactory(cfg), catfile.NewCache(cfg), repoProto, cfg)
+	catfileCache := catfile.NewCache(cfg)
+	defer catfileCache.Stop()
+
+	repo := localrepo.New(git.NewExecCommandFactory(cfg), catfileCache, repoProto, cfg)
 	headCommit, err := repo.ReadCommit(ctx, "HEAD")
 	require.NoError(t, err)
 	annotatedTagID, err := repo.WriteTag(ctx, git.ObjectID(headCommit.Id), "commit", []byte("annotated"), []byte("message"), gittest.TestUser, time.Now())
diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go
index 381a368ec..d1540f1a0 100644
--- a/internal/gitaly/service/ref/refs_test.go
+++ b/internal/gitaly/service/ref/refs_test.go
@@ -1081,6 +1081,8 @@ func TestFindTagNestedTag(t *testing.T) {
 			testhelper.MustRunCommand(t, tags, "xargs", cfg.Git.BinPath, "-C", repoPath, "tag", "-d")
 
 			catfileCache := catfile.NewCache(cfg)
+			defer catfileCache.Stop()
+
 			batch, err := catfileCache.BatchProcess(ctx, repo)
 			require.NoError(t, err)
 
diff --git a/internal/gitaly/service/repository/cleanup_test.go b/internal/gitaly/service/repository/cleanup_test.go
index 3e8b2ce0b..c5d53b73f 100644
--- a/internal/gitaly/service/repository/cleanup_test.go
+++ b/internal/gitaly/service/repository/cleanup_test.go
@@ -153,7 +153,10 @@ func TestRemoveWorktree(t *testing.T) {
 
 	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
-	repo := localrepo.New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg)
+	catfileCache := catfile.NewCache(cfg)
+	defer catfileCache.Stop()
+
+	repo := localrepo.New(gitCmdFactory, catfileCache, repoProto, cfg)
 
 	existingWorktreePath := filepath.Join(repoPath, worktreePrefix, "existing")
 	gittest.AddWorktree(t, cfg, repoPath, existingWorktreePath)
diff --git a/internal/gitaly/service/repository/clone_from_pool_internal_test.go b/internal/gitaly/service/repository/clone_from_pool_internal_test.go
index 3a99242c7..72f17d230 100644
--- a/internal/gitaly/service/repository/clone_from_pool_internal_test.go
+++ b/internal/gitaly/service/repository/clone_from_pool_internal_test.go
@@ -26,12 +26,14 @@ func newTestObjectPool(t *testing.T, cfg config.Cfg) (*objectpool.ObjectPool, *g
 	repo := gittest.InitRepoDir(t, cfg.Storages[0].Path, relativePath)
 
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
 
 	pool, err := objectpool.NewObjectPool(
 		cfg,
 		config.NewLocator(cfg),
 		gitCmdFactory,
-		catfile.NewCache(cfg),
+		catfileCache,
 		transaction.NewManager(cfg, backchannel.NewRegistry()),
 		repo.GetStorageName(),
 		relativePath,
diff --git a/internal/gitaly/service/repository/fork_test.go b/internal/gitaly/service/repository/fork_test.go
index dba63ffa2..8c7876882 100644
--- a/internal/gitaly/service/repository/fork_test.go
+++ b/internal/gitaly/service/repository/fork_test.go
@@ -226,6 +226,7 @@ func runSecureServer(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) s
 	), cfg)
 	gitCmdFactory := git.NewExecCommandFactory(cfg)
 	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
 
 	gitalypb.RegisterRepositoryServiceServer(server, NewServer(cfg, rubySrv, locator, txManager, gitCmdFactory, catfileCache))
 	gitalypb.RegisterHookServiceServer(server, hookservice.NewServer(cfg, hookManager, gitCmdFactory, nil))
diff --git a/internal/gitaly/service/repository/search_files_test.go b/internal/gitaly/service/repository/search_files_test.go
index 096485610..425c805cb 100644
--- a/internal/gitaly/service/repository/search_files_test.go
+++ b/internal/gitaly/service/repository/search_files_test.go
@@ -207,6 +207,8 @@ func TestSearchFilesByContentFailure(t *testing.T) {
 	t.Parallel()
 	cfg, repo, _ := testcfg.BuildWithRepo(t)
 	gitCommandFactory := git.NewExecCommandFactory(cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
 
 	server := NewServer(
 		cfg,
@@ -214,7 +216,7 @@ func TestSearchFilesByContentFailure(t *testing.T) {
 		config.NewLocator(cfg),
 		transaction.NewManager(cfg, backchannel.NewRegistry()),
 		gitCommandFactory,
-		catfile.NewCache(cfg),
+		catfileCache,
 	)
 
 	testCases := []struct {
@@ -330,6 +332,8 @@ func TestSearchFilesByNameFailure(t *testing.T) {
 	t.Parallel()
 	cfg := testcfg.Build(t)
 	gitCommandFactory := git.NewExecCommandFactory(cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
 
 	server := NewServer(
 		cfg,
@@ -337,7 +341,7 @@ func TestSearchFilesByNameFailure(t *testing.T) {
 		config.NewLocator(cfg),
 		transaction.NewManager(cfg, backchannel.NewRegistry()),
 		gitCommandFactory,
-		catfile.NewCache(cfg),
+		catfileCache,
 	)
 
 	testCases := []struct {
diff --git a/internal/gitaly/service/repository/snapshot_test.go b/internal/gitaly/service/repository/snapshot_test.go
index 973eb93b6..a4727957a 100644
--- a/internal/gitaly/service/repository/snapshot_test.go
+++ b/internal/gitaly/service/repository/snapshot_test.go
@@ -133,6 +133,7 @@ func TestGetSnapshotWithDedupe(t *testing.T) {
 
 			locator := config.NewLocator(cfg)
 			catfileCache := catfile.NewCache(cfg)
+			defer catfileCache.Stop()
 
 			// ensure commit cannot be found in current repository
 			c, err := catfileCache.BatchProcess(ctx, repo)
diff --git a/internal/middleware/commandstatshandler/commandstatshandler_test.go b/internal/middleware/commandstatshandler/commandstatshandler_test.go
index 184121228..d474744bd 100644
--- a/internal/middleware/commandstatshandler/commandstatshandler_test.go
+++ b/internal/middleware/commandstatshandler/commandstatshandler_test.go
@@ -51,13 +51,15 @@ func createNewServer(t *testing.T, cfg config.Cfg) *grpc.Server {
 	server := grpc.NewServer(opts...)
 
 	gitCommandFactory := git.NewExecCommandFactory(cfg)
+	catfileCache := catfile.NewCache(cfg)
+	t.Cleanup(catfileCache.Stop)
 
 	gitalypb.RegisterRefServiceServer(server, ref.NewServer(
 		cfg,
 		config.NewLocator(cfg),
 		gitCommandFactory,
 		transaction.NewManager(cfg, backchannel.NewRegistry()),
-		catfile.NewCache(cfg),
+		catfileCache,
 	))
 
 	return server