fix(colorscheme): resolve callback unregister race condition and CI test env

commit: 3b58ec414f4f2e5bfd079fd34c7eac48a41c04d4
author: bnema · 9mo ago · 2025-12-30 04:03 UTC
parent: 77255a2
signature: GPG Unverified2B82207D370127A0 The key is unknown

2 files · +20 -8

M .github/workflows/ci.yml +3 -0


@@ -103,4 +103,7 @@ jobs:
103	103		- name: Build frontend assets
104	104		run: make build-frontend
105	105		- name: Run tests
	106	+	env:
	107	+	GDK_BACKEND: wayland
	108	+	WAYLAND_DISPLAY: wayland-0
106	109		run: go test -v ./...


@@ -103,4 +103,7 @@ jobs:
103	- name: Build frontend assets	103		- name: Build frontend assets
104	run: make build-frontend	104		run: make build-frontend
105	- name: Run tests	105		- name: Run tests
		106	+	env:
		107	+	GDK_BACKEND: wayland
		108	+	WAYLAND_DISPLAY: wayland-0
106	run: go test -v ./...	109		run: go test -v ./...

M internal/infrastructure/colorscheme/resolver.go +17 -8


@@ -22,6 +22,11 @@ type ConfigProvider interface {
22	22		GetColorScheme() string
23	23		}
24	24
	25	+	// callbackWrapper wraps a callback function to enable pointer comparison for removal.
	26	+	type callbackWrapper struct {
	27	+	fn func(port.ColorSchemePreference)
	28	+	}
	29	+
25	30		// Resolver implements port.ColorSchemeResolver.
26	31		// It manages multiple detectors and respects config overrides.
27	32		type Resolver struct {

@@ -29,7 +34,7 @@ type Resolver struct {
29	34		config ConfigProvider
30	35		detectors []port.ColorSchemeDetector
31	36		current port.ColorSchemePreference
32		-	callbacks []func(port.ColorSchemePreference)
	37	+	callbacks []*callbackWrapper
33	38		}
34	39
35	40		// NewResolver creates a new color scheme resolver.

@@ -118,13 +123,13 @@ func (r *Resolver) Refresh() port.ColorSchemePreference {
118	123		if newPref.PrefersDark != r.current.PrefersDark {
119	124		r.current = newPref
120	125		// Copy callbacks to avoid holding lock during callback invocation
121		-	callbacks := make([]func(port.ColorSchemePreference), len(r.callbacks))
	126	+	callbacks := make([]*callbackWrapper, len(r.callbacks))
122	127		copy(callbacks, r.callbacks)
123	128
124	129		// Invoke callbacks outside of lock
125	130		r.mu.Unlock()
126	131		for _, cb := range callbacks {
127		-	cb(newPref)
	132	+	cb.fn(newPref)
128	133		}
129	134		r.mu.Lock()
130	135		} else {

@@ -139,17 +144,21 @@ func (r *Resolver) OnChange(callback func(port.ColorSchemePreference)) func() {
139	144		r.mu.Lock()
140	145		defer r.mu.Unlock()
141	146
142		-	r.callbacks = append(r.callbacks, callback)
143		-	idx := len(r.callbacks) - 1
	147	+	// Wrap callback to enable pointer comparison for removal
	148	+	wrapper := &callbackWrapper{fn: callback}
	149	+	r.callbacks = append(r.callbacks, wrapper)
144	150
145	151		// Return unregister function
146	152		return func() {
147	153		r.mu.Lock()
148	154		defer r.mu.Unlock()
149	155
150		-	// Remove callback by setting to nil and compacting later
151		-	if idx < len(r.callbacks) {
152		-	r.callbacks = append(r.callbacks[:idx], r.callbacks[idx+1:]...)
	156	+	// Find and remove callback by pointer equality
	157	+	for i, cb := range r.callbacks {
	158	+	if cb == wrapper {
	159	+	r.callbacks = append(r.callbacks[:i], r.callbacks[i+1:]...)
	160	+	return
	161	+	}
153	162		}
154	163		}
155	164		}

 	GetColorScheme() string
 }
+// callbackWrapper wraps a callback function to enable pointer comparison for removal.
+type callbackWrapper struct {
+	fn func(port.ColorSchemePreference)
+}
 // Resolver implements port.ColorSchemeResolver.
 // It manages multiple detectors and respects config overrides.
 type Resolver struct {
 	config    ConfigProvider
 	detectors []port.ColorSchemeDetector
 	current   port.ColorSchemePreference
-	callbacks []func(port.ColorSchemePreference)
+	callbacks []*callbackWrapper
 }
 // NewResolver creates a new color scheme resolver.
 	if newPref.PrefersDark != r.current.PrefersDark {
 		r.current = newPref
 		// Copy callbacks to avoid holding lock during callback invocation
-		callbacks := make([]func(port.ColorSchemePreference), len(r.callbacks))
+		callbacks := make([]*callbackWrapper, len(r.callbacks))
 		copy(callbacks, r.callbacks)
 		// Invoke callbacks outside of lock
 		r.mu.Unlock()
 		for _, cb := range callbacks {
-			cb(newPref)
+			cb.fn(newPref)
 		}
 		r.mu.Lock()
 	} else {
 	r.mu.Lock()
 	defer r.mu.Unlock()
-	r.callbacks = append(r.callbacks, callback)
+	// Wrap callback to enable pointer comparison for removal
-	idx := len(r.callbacks) - 1
+	wrapper := &callbackWrapper{fn: callback}
+	r.callbacks = append(r.callbacks, wrapper)
 	// Return unregister function
 	return func() {
 		r.mu.Lock()
 		defer r.mu.Unlock()
-		// Remove callback by setting to nil and compacting later
+		// Find and remove callback by pointer equality
-		if idx < len(r.callbacks) {
+		for i, cb := range r.callbacks {
-			r.callbacks = append(r.callbacks[:idx], r.callbacks[idx+1:]...)
+			if cb == wrapper {
+				r.callbacks = append(r.callbacks[:i], r.callbacks[i+1:]...)
+				return
+			}
 		}
 	}
 }