fix(core): add thread-safety to global Core instance (#95)

Protect the global `instance` variable with sync.RWMutex to prevent
data races when SetInstance/App() are called concurrently (especially
in tests).

Changes:
- Add instanceMu mutex to protect instance variable
- Update App() to use RLock for reading
- Update SetInstance() to use Lock for writing
- Add GetInstance() for non-panicking access
- Add ClearInstance() for test cleanup
- Update tests to use new thread-safe functions
- Add concurrent access test with race detector

Closes #84

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Snider 2026-02-01 16:03:44 +00:00 committed by GitHub
parent f2bbb71875
commit 04e70d9cda
3 changed files with 73 additions and 9 deletions

View file

@ -332,16 +332,38 @@ func MustServiceFor[T any](c *Core, name string) T {
// It panics if the Core has not been initialized via SetInstance.
// This is typically used by GUI runtimes that need global access.
func App() any {
if instance == nil {
instanceMu.RLock()
inst := instance
instanceMu.RUnlock()
if inst == nil {
panic("core.App() called before core.SetInstance()")
}
return instance.App
return inst.App
}
// SetInstance sets the global Core instance for App() access.
// This is typically called by GUI runtimes during initialization.
func SetInstance(c *Core) {
instanceMu.Lock()
instance = c
instanceMu.Unlock()
}
// GetInstance returns the global Core instance, or nil if not set.
// Use this for non-panicking access to the global instance.
func GetInstance() *Core {
instanceMu.RLock()
inst := instance
instanceMu.RUnlock()
return inst
}
// ClearInstance resets the global Core instance to nil.
// This is primarily useful for testing to ensure a clean state between tests.
func ClearInstance() {
instanceMu.Lock()
instance = nil
instanceMu.Unlock()
}
// Config returns the registered Config service.

View file

@ -89,18 +89,18 @@ func TestCore_App_Good(t *testing.T) {
assert.NoError(t, err)
// To test the global App() function, we need to set the global instance.
originalInstance := instance
instance = c
defer func() { instance = originalInstance }()
originalInstance := GetInstance()
SetInstance(c)
defer SetInstance(originalInstance)
assert.Equal(t, app, App())
}
func TestCore_App_Ugly(t *testing.T) {
// This test ensures that calling App() before the core is initialized panics.
originalInstance := instance
instance = nil
defer func() { instance = originalInstance }()
originalInstance := GetInstance()
ClearInstance()
defer SetInstance(originalInstance)
assert.Panics(t, func() {
App()
})
@ -295,3 +295,42 @@ func TestCore_WithName_Bad(t *testing.T) {
assert.Error(t, err)
assert.ErrorIs(t, err, assert.AnError)
}
func TestCore_GlobalInstance_ThreadSafety_Good(t *testing.T) {
// Save original instance
original := GetInstance()
defer SetInstance(original)
// Test SetInstance/GetInstance
c1, _ := New()
SetInstance(c1)
assert.Equal(t, c1, GetInstance())
// Test ClearInstance
ClearInstance()
assert.Nil(t, GetInstance())
// Test concurrent access (race detector should catch issues)
c2, _ := New(WithApp(&mockApp{}))
done := make(chan bool)
for i := 0; i < 10; i++ {
go func() {
SetInstance(c2)
_ = GetInstance()
done <- true
}()
go func() {
inst := GetInstance()
if inst != nil {
_ = inst.App
}
done <- true
}()
}
// Wait for all goroutines
for i := 0; i < 20; i++ {
<-done
}
}

View file

@ -90,7 +90,10 @@ type Core struct {
stoppables []Stoppable
}
var instance *Core
var (
instance *Core
instanceMu sync.RWMutex
)
// Config provides access to application configuration.
type Config interface {