From 04e70d9cda9f7741ba81363e181f6273a7d655df Mon Sep 17 00:00:00 2001
From: Snider <snider@host.uk.com>
Date: Sun, 1 Feb 2026 16:03:44 +0000
Subject: [PATCH] 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>
---
 pkg/framework/core/core.go       | 26 ++++++++++++++--
 pkg/framework/core/core_test.go  | 51 ++++++++++++++++++++++++++++----
 pkg/framework/core/interfaces.go |  5 +++-
 3 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/pkg/framework/core/core.go b/pkg/framework/core/core.go
index ac9997b9..2f0f70c7 100644
--- a/pkg/framework/core/core.go
+++ b/pkg/framework/core/core.go
@@ -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.
diff --git a/pkg/framework/core/core_test.go b/pkg/framework/core/core_test.go
index 6dbdaec6..1af883b8 100644
--- a/pkg/framework/core/core_test.go
+++ b/pkg/framework/core/core_test.go
@@ -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
+	}
+}
diff --git a/pkg/framework/core/interfaces.go b/pkg/framework/core/interfaces.go
index 5ee74143..069d8368 100644
--- a/pkg/framework/core/interfaces.go
+++ b/pkg/framework/core/interfaces.go
@@ -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 {