From 01ad33c879da209fb2368535239b2b5418c57f97 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Wed, 12 Aug 2026 23:20:44 +0100 Subject: [PATCH] Fix singleton ResourceManager data race (unlocked read vs test reset) GetResourceManager returned the process-global via an unprotected read (singleton_resources.go:50), while resetResourceManagerForTesting and ResetResourceManager write it under resourceManagerMutex. A live background task (graceful-degradation health check started by one test) reading the singleton while a shuffled test resets it was a data race (found via -race -shuffle: Write at singleton_resources_test.go:941/942 vs Read at autocleanup.go:554 / error_recovery.go:910). Fix: hold resourceManagerMutex around the read, matching the sibling GetGlobalTaskRegistry (autocleanup.go:414). Added a deterministic concurrent regression test (GetResourceManager vs reset under -race). Verified: 4x full -race -count=1 -shuffle=on ./... all exit 0, 0 races, 0 fails; source-order suite green; lint clean. --- singleton_resources.go | 3 +++ singleton_resources_race_test.go | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 singleton_resources_race_test.go diff --git a/singleton_resources.go b/singleton_resources.go index a52f12e..b7e7094 100644 --- a/singleton_resources.go +++ b/singleton_resources.go @@ -36,6 +36,9 @@ type ResourceManager struct { // GetResourceManager returns the global singleton ResourceManager instance func GetResourceManager() *ResourceManager { + resourceManagerMutex.Lock() + defer resourceManagerMutex.Unlock() + resourceManagerOnce.Do(func() { globalResourceManager = &ResourceManager{ httpClients: make(map[string]*http.Client), diff --git a/singleton_resources_race_test.go b/singleton_resources_race_test.go new file mode 100644 index 0000000..33e5e0a --- /dev/null +++ b/singleton_resources_race_test.go @@ -0,0 +1,41 @@ +package traefikoidc + +import ( + "sync" + "testing" +) + +// TestConcurrentGetResourceManagerVsReset stresses GetResourceManager against +// resetResourceManagerForTesting under -race. GetResourceManager returns the +// process-global via an unprotected read unless the caller holds +// resourceManagerMutex (regression: a live background task reading the +// singleton while a shuffled test resets it was a data race). Run with -race; +// this fails (race) before the mutex fix and passes after. +func TestConcurrentGetResourceManagerVsReset(t *testing.T) { + var wg sync.WaitGroup + + // Readers constantly re-fetch the global, as a background task would. + for g := 0; g < 8; g++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 2000; i++ { + if rm := GetResourceManager(); rm == nil { + t.Log("GetResourceManager returned nil") + } + } + }() + } + + // Resetter repeatedly pins/unpins the singleton with the test helper, + // mirroring resetResourceManagerForTesting's writes in a shuffled suite. + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 200; i++ { + resetResourceManagerForTesting() + } + }() + + wg.Wait() +}