diff --git a/refresh_coordinator.go b/refresh_coordinator.go index ca14ba2..a037df0 100644 --- a/refresh_coordinator.go +++ b/refresh_coordinator.go @@ -37,6 +37,11 @@ type RefreshCoordinator struct { stopChan chan struct{} config RefreshCoordinatorConfig wg sync.WaitGroup + // shutdownOnce makes Shutdown idempotent: close(rc.stopChan) on an + // already-closed channel would panic ('close of closed channel'). + // Repeated setup/teardown (e.g. Traefik plugin reload) can close the + // same Authenticator -> same coordinator more than once. + shutdownOnce sync.Once } // RefreshCoordinatorConfig configures the refresh coordinator behavior @@ -720,8 +725,10 @@ func (rc *RefreshCoordinator) GetMetrics() map[string]interface{} { // (one map LoadAndDelete) and harmless after Shutdown — sync.Map operations // remain safe on an unused coordinator until GC. func (rc *RefreshCoordinator) Shutdown() { - close(rc.stopChan) - rc.wg.Wait() + rc.shutdownOnce.Do(func() { + close(rc.stopChan) + rc.wg.Wait() + }) } // AllowRequest reports whether the circuit breaker allows a request. Lock-free. diff --git a/review_r58_shutdown_test.go b/review_r58_shutdown_test.go new file mode 100644 index 0000000..d8565da --- /dev/null +++ b/review_r58_shutdown_test.go @@ -0,0 +1,31 @@ +package traefikoidc + +import ( + "testing" + "time" +) + +// TestRefreshCoordinatorShutdownIdempotent guards against a panic when the +// coordinator is shut down more than once. Close() on the same +// Authenticator can be reached via both the plugin-context cancellation hook +// (main.go) and the resource-manager teardown path, so RefreshCoordinator +// Shutdown must tolerate repeated calls — the previous close(rc.stopChan) +// panicked with 'close of closed channel' on the second call. +func TestRefreshCoordinatorShutdownIdempotent(t *testing.T) { + rc := NewRefreshCoordinator(DefaultRefreshCoordinatorConfig(), NewLogger(DefaultLogLevel)) + defer func() { + if r := recover(); r != nil { + t.Fatalf("second Shutdown panicked: %v", r) + } + }() + rc.Shutdown() + rc.Shutdown() +} + +// Ensure cleanup goroutine is running before shutdown completes so the +// shutdown path is actually exercised (stopChan close) and wg drained. +func TestRefreshCoordinatorShutdownStopsCleanup(t *testing.T) { + rc := NewRefreshCoordinator(DefaultRefreshCoordinatorConfig(), NewLogger(DefaultLogLevel)) + time.Sleep(50 * time.Millisecond) + rc.Shutdown() +}