From 9e0fff5e2241576b33b60fffd7cf853f474e0239 Mon Sep 17 00:00:00 2001
From: Egon Elbre <egonelbre@gmail.com>
Date: Wed, 1 Nov 2023 19:06:33 +0200
Subject: [PATCH] satellite/gcbf: fix data race in TestGCBFUseRangedLoop

Satellites[0].GCBF is already started when testplanet boots up,
so calling Run separately ends up causing a data race.

Instead create a new instance, that should avoid this issue.

Fixes https://github.com/storj/storj/issues/6435

Change-Id: I6603ef63da7a6ab8bdb952cf5aaca17eb0392e2c
---
 satellite/gc-bf.go      |  8 ++++----
 satellite/gc-bf_test.go | 30 +++++++++++++++++++++++-------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/satellite/gc-bf.go b/satellite/gc-bf.go
index 0b5fe2fd9..2a15810d9 100644
--- a/satellite/gc-bf.go
+++ b/satellite/gc-bf.go
@@ -91,14 +91,14 @@ func NewGarbageCollectionBF(log *zap.Logger, db DB, metabaseDB *metabase.DB, rev
 		peer.GarbageCollection.Config = config.GarbageCollectionBF
 
 		var observer rangedloop.Observer
-		if config.GarbageCollectionBF.UseSyncObserver {
+		if peer.GarbageCollection.Config.UseSyncObserver {
 			observer = bloomfilter.NewSyncObserver(log.Named("gc-bf"),
-				config.GarbageCollectionBF,
+				peer.GarbageCollection.Config,
 				peer.Overlay.DB,
 			)
 		} else {
 			observer = bloomfilter.NewObserver(log.Named("gc-bf"),
-				config.GarbageCollectionBF,
+				peer.GarbageCollection.Config,
 				peer.Overlay.DB,
 			)
 		}
@@ -109,7 +109,7 @@ func NewGarbageCollectionBF(log *zap.Logger, db DB, metabaseDB *metabase.DB, rev
 			observer,
 		})
 
-		if !config.GarbageCollectionBF.RunOnce {
+		if !peer.GarbageCollection.Config.RunOnce {
 			peer.Services.Add(lifecycle.Item{
 				Name:  "garbage-collection-bf",
 				Run:   peer.RangedLoop.Service.Run,
diff --git a/satellite/gc-bf_test.go b/satellite/gc-bf_test.go
index 6c2a00bf4..e06121d26 100644
--- a/satellite/gc-bf_test.go
+++ b/satellite/gc-bf_test.go
@@ -7,9 +7,9 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"go.uber.org/zap"
 
 	"storj.io/common/testcontext"
+	"storj.io/storj/private/revocation"
 	"storj.io/storj/private/testplanet"
 	"storj.io/storj/satellite"
 )
@@ -17,13 +17,29 @@ import (
 func TestGCBFUseRangedLoop(t *testing.T) {
 	testplanet.Run(t, testplanet.Config{
 		SatelliteCount: 1,
-		Reconfigure: testplanet.Reconfigure{
-			Satellite: func(log *zap.Logger, index int, config *satellite.Config) {
-				config.GarbageCollectionBF.RunOnce = true
-			},
-		},
 	}, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) {
-		err := planet.Satellites[0].GCBF.Run(ctx)
+		config := planet.Satellites[0].Config
+
+		revocationDB, err := revocation.OpenDBFromCfg(ctx, config.Server.Config)
+		require.NoError(t, err)
+		defer ctx.Check(revocationDB.Close)
+
+		config.GarbageCollectionBF.RunOnce = true
+
+		gcbf, err := satellite.NewGarbageCollectionBF(
+			planet.Log().Named("test-gcbf"),
+			// hopefully we can share the databases
+			planet.Satellites[0].GCBF.DB,
+			planet.Satellites[0].Metabase.DB,
+			revocationDB,
+			planet.NewVersionInfo(),
+			&config,
+			nil,
+		)
+		require.NoError(t, err)
+		defer ctx.Check(gcbf.Close)
+
+		err = gcbf.Run(ctx)
 		require.NoError(t, err)
 	})
 }