fix: stabilize federated agent runtime updates (#550)

go-gost/x/config/config.go

@@ -3,6 +3,7 @@ package config
 import (
 	"encoding/json"
 	"io"
+	"reflect"
 	"sync"
 	"time"
 
@@ -30,9 +31,87 @@ func Global() *Config {
 	globalMux.RLock()
 	defer globalMux.RUnlock()
 
-	cfg := &Config{}
-	*cfg = *global
-	return cfg
+	return cloneConfig(global)
+}
+
+// cloneConfig returns a detached snapshot of the runtime config. Runtime
+// commands mutate slices, pointers and metadata maps, so a shallow struct copy
+// is not sufficient once readers and persistence run concurrently.
+func cloneConfig(c *Config) *Config {
+	if c == nil {
+		return nil
+	}
+	v := cloneConfigValue(reflect.ValueOf(c))
+	if !v.IsValid() || v.IsNil() {
+		return nil
+	}
+	return v.Interface().(*Config)
+}
+
+func cloneConfigValue(v reflect.Value) reflect.Value {
+	if !v.IsValid() {
+		return reflect.Value{}
+	}
+
+	switch v.Kind() {
+	case reflect.Interface:
+		if v.IsNil() {
+			return reflect.Zero(v.Type())
+		}
+		cloned := cloneConfigValue(v.Elem())
+		out := reflect.New(v.Type()).Elem()
+		if cloned.IsValid() && cloned.Type().AssignableTo(v.Type()) {
+			out.Set(cloned)
+		} else if cloned.IsValid() && cloned.Type().Implements(v.Type()) {
+			out.Set(cloned)
+		} else if cloned.IsValid() {
+			out.Set(cloned)
+		}
+		return out
+	case reflect.Pointer:
+		if v.IsNil() {
+			return reflect.Zero(v.Type())
+		}
+		out := reflect.New(v.Type().Elem())
+		out.Elem().Set(cloneConfigValue(v.Elem()))
+		return out
+	case reflect.Map:
+		if v.IsNil() {
+			return reflect.Zero(v.Type())
+		}
+		out := reflect.MakeMapWithSize(v.Type(), v.Len())
+		iter := v.MapRange()
+		for iter.Next() {
+			out.SetMapIndex(cloneConfigValue(iter.Key()), cloneConfigValue(iter.Value()))
+		}
+		return out
+	case reflect.Slice:
+		if v.IsNil() {
+			return reflect.Zero(v.Type())
+		}
+		out := reflect.MakeSlice(v.Type(), v.Len(), v.Len())
+		for i := 0; i < v.Len(); i++ {
+			out.Index(i).Set(cloneConfigValue(v.Index(i)))
+		}
+		return out
+	case reflect.Array:
+		out := reflect.New(v.Type()).Elem()
+		for i := 0; i < v.Len(); i++ {
+			out.Index(i).Set(cloneConfigValue(v.Index(i)))
+		}
+		return out
+	case reflect.Struct:
+		out := reflect.New(v.Type()).Elem()
+		out.Set(v)
+		for i := 0; i < v.NumField(); i++ {
+			if out.Field(i).CanSet() && v.Field(i).CanInterface() {
+				out.Field(i).Set(cloneConfigValue(v.Field(i)))
+			}
+		}
+		return out
+	default:
+		return v
+	}
 }
 
 func Set(c *Config) {

go-gost/x/config/config_concurrency_test.go

@@ -0,0 +1,121 @@
+package config
+
+import (
+	"encoding/json"
+	"path/filepath"
+	"sync"
+	"testing"
+)
+
+func TestGlobalReturnsDetachedSnapshot(t *testing.T) {
+	original := Global()
+	t.Cleanup(func() { Set(original) })
+
+	Set(&Config{Services: []*ServiceConfig{{
+		Name:     "snapshot-service",
+		Metadata: map[string]any{"paused": false},
+		Handler: &HandlerConfig{
+			Type:     "relay",
+			Metadata: map[string]any{"retries": 2},
+		},
+	}}})
+
+	snapshot := Global()
+	snapshot.Services[0].Name = "changed"
+	snapshot.Services[0].Metadata["paused"] = true
+	snapshot.Services[0].Handler.Metadata["retries"] = 9
+
+	current := Global()
+	if current.Services[0].Name != "snapshot-service" {
+		t.Fatalf("snapshot mutated global service name: %q", current.Services[0].Name)
+	}
+	if paused, _ := current.Services[0].Metadata["paused"].(bool); paused {
+		t.Fatalf("snapshot mutated global service metadata")
+	}
+	if retries, _ := current.Services[0].Handler.Metadata["retries"].(int); retries != 2 {
+		t.Fatalf("snapshot mutated nested handler metadata: %v", current.Services[0].Handler.Metadata["retries"])
+	}
+}
+
+func TestConcurrentGlobalSnapshotAndUpdate(t *testing.T) {
+	original := Global()
+	t.Cleanup(func() { Set(original) })
+
+	Set(&Config{Services: []*ServiceConfig{{
+		Name:     "concurrent-service",
+		Metadata: map[string]any{"generation": 0},
+	}}})
+
+	var wg sync.WaitGroup
+	for worker := 0; worker < 4; worker++ {
+		wg.Add(1)
+		go func(worker int) {
+			defer wg.Done()
+			for i := 0; i < 500; i++ {
+				if err := OnUpdate(func(c *Config) error {
+					c.Services[0] = &ServiceConfig{
+						Name:     "concurrent-service",
+						Metadata: map[string]any{"generation": worker*500 + i},
+					}
+					return nil
+				}); err != nil {
+					t.Errorf("OnUpdate: %v", err)
+					return
+				}
+			}
+		}(worker)
+	}
+
+	for i := 0; i < 2000; i++ {
+		if _, err := json.Marshal(Global()); err != nil {
+			t.Fatalf("marshal snapshot: %v", err)
+		}
+	}
+	wg.Wait()
+}
+
+func TestConcurrentPersistProducesValidConfig(t *testing.T) {
+	original := Global()
+	originalPath := PersistPath()
+	persistMu.Lock()
+	originalEnabled := persistEnable
+	persistMu.Unlock()
+	t.Cleanup(func() {
+		Set(original)
+		SetPersistPath(originalPath)
+		persistMu.Lock()
+		persistEnable = originalEnabled
+		persistMu.Unlock()
+	})
+
+	path := filepath.Join(t.TempDir(), "gost.json")
+	SetPersistPath(path)
+	EnablePersist()
+	Set(&Config{Services: []*ServiceConfig{{Name: "persist-service"}}})
+
+	var wg sync.WaitGroup
+	for worker := 0; worker < 4; worker++ {
+		wg.Add(1)
+		go func(worker int) {
+			defer wg.Done()
+			for i := 0; i < 50; i++ {
+				if err := OnUpdate(func(c *Config) error {
+					c.Services[0].Metadata = map[string]any{"generation": worker*50 + i}
+					return nil
+				}); err != nil {
+					t.Errorf("persist update: %v", err)
+					return
+				}
+			}
+		}(worker)
+	}
+	wg.Wait()
+
+	var persisted Config
+	if err := persisted.ReadFile(path); err != nil {
+		t.Fatalf("read persisted config: %v", err)
+	}
+	if len(persisted.Services) != 1 || persisted.Services[0] == nil || persisted.Services[0].Name != "persist-service" {
+		t.Fatalf("unexpected persisted services: %#v", persisted.Services)
+	}
+}

go-gost/x/config/persist.go

@@ -42,9 +42,9 @@ func EnablePersist() {
 // persist writes the current global config to the configured file atomically.
 func persist() error {
 	persistMu.Lock()
+	defer persistMu.Unlock()
 	path := persistPath
 	enabled := persistEnable
-	persistMu.Unlock()
 
 	if !enabled || path == "" {
 		return nil