fix data race issue from test

Author: wbrowne

backend/instancemgmt/instance_manager.go

@@ -18,9 +18,10 @@ var (
 		Name:      "active_instances",
 		Help:      "The number of active plugin instances",
 	})
-	disposeTTL = 5 * time.Second
 )
 
+const defaultDisposeTTL = 5 * time.Second // Time to wait before disposing an instance
+
 // Instance is a marker interface for an instance.
 type Instance interface{}
 
@@ -73,21 +74,33 @@ type InstanceProvider interface {
 
 // New create a new instance manager.
 func New(provider InstanceProvider) InstanceManager {
+	return newInstanceManager(provider, defaultDisposeTTL)
+}
+
+func newInstanceManager(provider InstanceProvider, disposeTTL time.Duration) *instanceManager {
 	if provider == nil {
 		panic("provider cannot be nil")
 	}
 
+	if disposeTTL <= 0 {
+		disposeTTL = defaultDisposeTTL
+	}
+
 	return &instanceManager{
-		provider: provider,
-		cache:    sync.Map{},
-		locker:   newLocker(),
+		provider:   provider,
+		cache:      sync.Map{},
+		locker:     newLocker(),
+		disposeTTL: disposeTTL,
 	}
 }
 
 type instanceManager struct {
-	locker   *locker
-	provider InstanceProvider
-	cache    sync.Map
+	locker     *locker
+	provider   InstanceProvider
+	cache      sync.Map
+	disposeTTL time.Duration
+
+	disposeMutex sync.Mutex // Mutex to protect disposeTTL access in tests
 }
 
 func (im *instanceManager) Get(ctx context.Context, pluginContext backend.PluginContext) (Instance, error) {
@@ -121,7 +134,7 @@ func (im *instanceManager) Get(ctx context.Context, pluginContext backend.Plugin
 		}
 
 		if disposer, valid := ci.instance.(InstanceDisposer); valid {
-			time.AfterFunc(disposeTTL, func() {
+			time.AfterFunc(im.disposeTTL, func() {
 				disposer.Dispose()
 				activeInstances.Dec()
 			})
@@ -157,6 +170,14 @@ func (im *instanceManager) Do(ctx context.Context, pluginContext backend.PluginC
 	return nil
 }
 
+// setDisposeTTL sets the TTL for disposing instances.
+// This method is only used for testing purposes to control the dispose timing behavior.
+func (im *instanceManager) setDisposeTTL(ttl time.Duration) {
+	im.disposeMutex.Lock()
+	defer im.disposeMutex.Unlock()
+	im.disposeTTL = ttl
+}
+
 func callInstanceHandlerFunc(fn InstanceCallbackFunc, instance interface{}) {
 	var params = []reflect.Value{}
 	params = append(params, reflect.ValueOf(instance))

backend/instancemgmt/instance_manager_test.go

@@ -22,7 +22,7 @@ func TestInstanceManager(t *testing.T) {
 	}
 
 	tip := &testInstanceProvider{}
-	im := New(tip)
+	im := newInstanceManager(tip, time.Millisecond)
 
 	t.Run("When getting instance should create a new instance", func(t *testing.T) {
 		instance, err := im.Get(ctx, pCtx)
@@ -44,11 +44,6 @@ func TestInstanceManager(t *testing.T) {
 					Updated: time.Now(),
 				},
 			}
-			origDisposeTTL := disposeTTL
-			disposeTTL = time.Millisecond
-			t.Cleanup(func() {
-				disposeTTL = origDisposeTTL
-			})
 			newInstance, err := im.Get(ctx, pCtxUpdated)
 
 			t.Run("New instance should be created", func(t *testing.T) {
@@ -111,12 +106,6 @@ func TestInstanceManagerConcurrency(t *testing.T) {
 	})
 
 	t.Run("Check possible race condition issues when re-creating instance on settings update", func(t *testing.T) {
-		origDisposeTTL := disposeTTL
-		disposeTTL = time.Millisecond
-		t.Cleanup(func() {
-			disposeTTL = origDisposeTTL
-		})
-
 		ctx := context.Background()
 		initialPCtx := backend.PluginContext{
 			OrgID: 1,
@@ -125,7 +114,8 @@ func TestInstanceManagerConcurrency(t *testing.T) {
 			},
 		}
 		tip := &testInstanceProvider{}
-		im := New(tip)
+		im := New(tip).(*instanceManager)
+		im.setDisposeTTL(time.Millisecond)
 		// Creating initial instance with old contexts
 		instanceToDispose, _ := im.Get(ctx, initialPCtx)
 

backend/instancemgmt/intance_manager_ttl.go

@@ -33,13 +33,9 @@ func newTTLInstanceManager(provider InstanceProvider, instanceTTL, instanceClean
 	cache.OnEvicted(func(_ string, value interface{}) {
 		ci := value.(CachedInstance)
 		if disposer, valid := ci.instance.(InstanceDisposer); valid {
-			time.AfterFunc(disposeTTL, func() {
-				disposer.Dispose()
-				activeInstances.Dec()
-			})
-		} else {
-			activeInstances.Dec()
+			disposer.Dispose()
 		}
+		activeInstances.Dec()
 	})
 
 	return &instanceManagerWithTTL{

backend/instancemgmt/intance_manager_ttl_test.go

@@ -43,11 +43,7 @@ func TestTTLInstanceManager(t *testing.T) {
 					Updated: time.Now(),
 				},
 			}
-			origDisposeTTL := disposeTTL
-			disposeTTL = time.Millisecond
-			t.Cleanup(func() {
-				disposeTTL = origDisposeTTL
-			})
+
 			newInstance, err := im.Get(ctx, pCtxUpdated)
 
 			t.Run("New instance should be created", func(t *testing.T) {
@@ -192,12 +188,6 @@ func TestTTLInstanceManagerConcurrency(t *testing.T) {
 	})
 
 	t.Run("Check possible race condition issues when re-creating instance on settings update", func(t *testing.T) {
-		origDisposeTTL := disposeTTL
-		disposeTTL = time.Millisecond
-		t.Cleanup(func() {
-			disposeTTL = origDisposeTTL
-		})
-
 		ctx := context.Background()
 		initialPCtx := backend.PluginContext{
 			OrgID: 1,