Fix goroutine issues with config

config/impl.go

@@ -8,9 +8,16 @@ import "path/filepath"
 import "github.com/fsnotify/fsnotify"
 import "git.tebibyte.media/tomo/tomo/event"
 
+// Goroutine model:
+// All private methods (except for lockAndProcessEvent) do not lock the config,
+// but all public methods do. Private methods may not call public methods.
+// Locking must always be method-level, with a call to Lock at the start,
+// directly followed by a deferred call to Unlock.
+
 type config struct {
 	open    bool
 	watcher *fsnotify.Watcher
+	lock    sync.RWMutex
 
 	paths struct {
 		user     string
@@ -19,12 +26,11 @@ type config struct {
 	}
 
 	data struct {
-		user *File
+		user   *File
 		system []map[string] Value
 	}
 	
 	on struct {
-		lock   sync.RWMutex
 		change event.Broadcaster[func (string)]
 	}
 }
@@ -45,10 +51,33 @@ func NewConfig (user string, system ...string) (ConfigCloser, error) {
 	conf.paths.system = system
 	err := conf.init()
 	if err != nil { return nil, err }
-	go conf.run()
+	go func () {
+		for event := range conf.watcher.Events {
+			conf.lockAndProcessEvent(event)
+		}
+	} ()
 	return conf, nil
 }
 
+// this method may only be run in the goroutine spawned by NewConfig.
+func (this *config) lockAndProcessEvent (event fsnotify.Event) {
+	this.lock.Lock()
+	defer this.lock.Unlock()
+	if !(event.Has(fsnotify.Write))                  { return }
+	if _, ok := this.paths.watching[event.Name]; !ok { return }
+
+	if event.Name == this.paths.user {
+		this.reloadUser()
+	} else {
+		index := slices.Index(this.paths.system, event.Name)
+		if index > 0 {
+			this.reloadSystem(index)
+		}
+	}
+
+	// TODO diff and call event handler if changed
+}
+
 func (this *config) init () error {
 	watcher, err := fsnotify.NewWatcher()
 	if err != nil { return err }
@@ -67,29 +96,6 @@ func (this *config) init () error {
 	return nil
 }
 
-func (this *config) run () {
-	for event := range this.watcher.Events {
-		if !(event.Has(fsnotify.Write))                  { continue }
-		if _, ok := this.paths.watching[event.Name]; !ok { continue }
-
-		if event.Name == this.paths.user {
-			this.reloadUser()
-		} else {
-			index := slices.Index(this.paths.system, event.Name)
-			if index > 0 {
-				this.reloadSystem(index)
-			}
-		}
-
-		// TODO diff and call event handler if changed
-	}
-}
-
-func (this *config) Close () error {
-	this.open = false
-	return this.watcher.Close()
-}
-
 func (this *config) errIfClosed () error {
 	if this.open {
 		return nil
@@ -140,7 +146,15 @@ func (this *config) saveUser () error {
 	return nil
 }
 
-func (this *config) Get (key string, fallback Value) (Value, error) {
+func (this *config) processUserDiff (changed []string) {
+	// TODO
+}
+
+func (this *config) processSystemDiff (index int, changed []string) {
+	// TODO
+}
+
+func (this *config) get (key string, fallback Value) (Value, error) {
 	// try user config
 	if !KeyValid(key) { return nil, ErrMalformedKey }
 
@@ -160,10 +174,26 @@ func (this *config) Get (key string, fallback Value) (Value, error) {
 
 	// use fallback
 	return fallback, nil
+	
+}
+
+func (this *config) Get (key string, fallback Value) (Value, error) {
+	this.lock.Lock()
+	defer this.lock.Unlock()
+	return this.get(key, fallback)
+}
+
+func (this *config) Close () error {
+	this.lock.Lock()
+	defer this.lock.Unlock()
+	this.open = false
+	return this.watcher.Close()
 }
 
 func (this *config) GetString (key string, fallback string) (string, error) {
-	value, err := this.Get(key, ValueString(fallback))
+	this.lock.Lock()
+	defer this.lock.Unlock()
+	value, err := this.get(key, ValueString(fallback))
 	if err != nil { return "", err }
 	if value, ok := value.(ValueString); ok {
 		return string(value), nil
@@ -172,7 +202,9 @@ func (this *config) GetString (key string, fallback string) (string, error) {
 }
 
 func (this *config) GetNumber (key string, fallback float64) (float64, error) {
-	value, err := this.Get(key, ValueNumber(fallback))
+	this.lock.Lock()
+	defer this.lock.Unlock()
+	value, err := this.get(key, ValueNumber(fallback))
 	if err != nil { return 0, err }
 	if value, ok := value.(ValueNumber); ok {
 		return float64(value), nil
@@ -181,7 +213,9 @@ func (this *config) GetNumber (key string, fallback float64) (float64, error) {
 }
 
 func (this *config) GetBool (key string, fallback bool) (bool, error) {
-	value, err := this.Get(key, ValueBool(fallback))
+	this.lock.Lock()
+	defer this.lock.Unlock()
+	value, err := this.get(key, ValueBool(fallback))
 	if err != nil { return false, err }
 	if value, ok := value.(ValueBool); ok {
 		return bool(value), nil
@@ -190,6 +224,8 @@ func (this *config) GetBool (key string, fallback bool) (bool, error) {
 }
 
 func (this *config) Set (key string, value Value) error {
+	this.lock.Lock()
+	defer this.lock.Unlock()
 	if this.data.user == nil { this.data.user = NewFile() }
 	err := this.data.user.Set(key, value)
 	if err != nil { return err }
@@ -197,6 +233,8 @@ func (this *config) Set (key string, value Value) error {
 }
 
 func (this *config) Reset (key string) error {
+	this.lock.Lock()
+	defer this.lock.Unlock()
 	if this.data.user == nil { this.data.user = NewFile() }
 	err := this.data.user.Reset(key)
 	if err != nil { return err }
@@ -204,7 +242,7 @@ func (this *config) Reset (key string) error {
 }
 
 func (this *config) OnChange (callback func (string)) event.Cookie {
-	this.on.lock.Lock()
-	defer this.on.lock.Unlock()
+	this.lock.Lock()
+	defer this.lock.Unlock()
 	return this.on.change.Connect(callback)
 }