Fix concurrent map read/write panic in configuration map

internal/go-configmap/cli.go

@@ -22,11 +22,14 @@ import (
 )
 
 func (c *Map) SetFromENV(key string, arg string) error {
+	c.ensureMux()
+	c.mux.Lock()
+	defer c.mux.Unlock()
+
 	// in case of schemaless configuration, we don't know the type of the setting
 	// we will save it as a string
 	if len(c.schema) == 0 {
-		c.Set(key, arg)
-		return nil
+		return c.setValue(key, arg)
 	}
 
 	// Find the correct type for the given setting
@@ -55,12 +58,16 @@ func (c *Map) SetFromENV(key string, arg string) error {
 		}
 	}
 
-	return c.Set(key, value)
+	return c.setValue(key, value)
 }
 
 func (c *Map) SetFromCLIArgs(key string, args ...string) error {
+	c.ensureMux()
+	c.mux.Lock()
+	defer c.mux.Unlock()
+
 	if len(args) == 0 {
-		c.Delete(key)
+		c.delete(strings.Split(key, "."))
 		return nil
 	}
 
@@ -69,9 +76,9 @@ func (c *Map) SetFromCLIArgs(key string, args ...string) error {
 	if len(c.schema) == 0 {
 		switch len(args) {
 		case 1:
-			c.Set(key, args[0])
+			c.setValue(key, args[0])
 		default:
-			c.Set(key, args)
+			c.setValue(key, args)
 		}
 		return nil
 	}
@@ -107,7 +114,7 @@ func (c *Map) SetFromCLIArgs(key string, args ...string) error {
 		return fmt.Errorf("error setting value: key is not an array, but multiple values were provided")
 	}
 
-	return c.Set(key, value)
+	return c.setValue(key, value)
 }
 
 func (c *Map) InjectEnvVars(env []string, prefix string) []error {

internal/go-configmap/configuration.go

@@ -19,19 +19,28 @@ import (
 	"fmt"
 	"reflect"
 	"strings"
+	"sync"
 
 	"fortio.org/safecast"
 )
 
 type Map struct {
 	values map[string]any
 	schema map[string]reflect.Type
+	mux    *sync.RWMutex
 }
 
 func New() *Map {
 	return &Map{
 		values: make(map[string]any),
 		schema: make(map[string]reflect.Type),
+		mux:    &sync.RWMutex{},
+	}
+}
+
+func (c *Map) ensureMux() {
+	if c.mux == nil {
+		c.mux = &sync.RWMutex{}
 	}
 }
 
@@ -41,8 +50,9 @@ func (c Map) Get(key string) any {
 }
 
 func (c Map) GetOk(key string) (any, bool) {
-	keys := strings.Split(key, ".")
-	return c.get(keys)
+	c.mux.RLock()
+	defer c.mux.RUnlock()
+	return c.get(strings.Split(key, "."))
 }
 
 func (c Map) get(keys []string) (any, bool) {
@@ -60,7 +70,7 @@ func (c Map) get(keys []string) (any, bool) {
 	return nil, false
 }
 
-func (c Map) Set(key string, value any) error {
+func (c Map) setValue(key string, value any) error {
 	if len(c.schema) > 0 {
 		t, ok := c.schema[key]
 		if !ok {
@@ -72,11 +82,16 @@ func (c Map) Set(key string, value any) error {
 		}
 		value = newValue
 	}
-	keys := strings.Split(key, ".")
-	c.set(keys, value)
+	c.set(strings.Split(key, "."), value)
 	return nil
 }
 
+func (c Map) Set(key string, value any) error {
+	c.mux.Lock()
+	defer c.mux.Unlock()
+	return c.setValue(key, value)
+}
+
 func tryConversion(current any, desiredType reflect.Type) (any, error) {
 	currentType := reflect.TypeOf(current)
 	if currentType == desiredType {
@@ -144,8 +159,9 @@ func (c Map) set(keys []string, value any) {
 }
 
 func (c Map) Delete(key string) {
-	keys := strings.Split(key, ".")
-	c.delete(keys)
+	c.mux.Lock()
+	defer c.mux.Unlock()
+	c.delete(strings.Split(key, "."))
 }
 
 func (c Map) delete(keys []string) {
@@ -167,10 +183,17 @@ func (c Map) delete(keys []string) {
 }
 
 func (c *Map) Merge(x *Map) error {
+	c.ensureMux()
+	c.mux.Lock()
+	defer c.mux.Unlock()
+	return c.merge(x)
+}
+
+func (c *Map) merge(x *Map) error {
 	for xk, xv := range x.values {
 		if xSubConf, ok := xv.(*Map); ok {
 			if subConf, ok := c.values[xk].(*Map); ok {
-				if err := subConf.Merge(xSubConf); err != nil {
+				if err := subConf.merge(xSubConf); err != nil {
 					return err
 				}
 				continue
@@ -191,10 +214,16 @@ func (c *Map) Merge(x *Map) error {
 }
 
 func (c *Map) AllKeys() []string {
+	c.ensureMux()
+	c.mux.RLock()
+	defer c.mux.RUnlock()
 	return c.allKeys("")
 }
 
 func (c *Map) Schema() map[string]reflect.Type {
+	c.ensureMux()
+	c.mux.RLock()
+	defer c.mux.RUnlock()
 	return c.schema
 }
 
@@ -217,5 +246,22 @@ func (c *Map) allKeys(prefix string) []string {
 }
 
 func (c *Map) SetKeyTypeSchema(key string, t any) {
+	c.ensureMux()
+	c.mux.Lock()
+	defer c.mux.Unlock()
 	c.schema[key] = reflect.TypeOf(t)
 }
+
+// deepSnapshot recursively builds a plain map[string]any, expanding all *Map
+// sub-objects into plain maps. Must be called while the top-level mux is held.
+func deepSnapshot(values map[string]any) map[string]any {
+	out := make(map[string]any, len(values))
+	for k, v := range values {
+		if subMap, ok := v.(*Map); ok {
+			out[k] = deepSnapshot(subMap.values)
+		} else {
+			out[k] = v
+		}
+	}
+	return out
+}

internal/go-configmap/configuration_test.go

@@ -17,6 +17,8 @@ package configmap_test
 
 import (
 	"encoding/json"
+	"fmt"
+	"sync"
 	"testing"
 
 	"github.com/arduino/arduino-cli/internal/go-configmap"
@@ -140,6 +142,19 @@ dir:
 	}
 }
 
+func TestConcurrentAccess(t *testing.T) {
+	c := configmap.New()
+	c.Set("updater.enable_notification", true)
+	var wg sync.WaitGroup
+	for i := range 50 {
+		wg.Add(3)
+		go func() { defer wg.Done(); _ = c.GetBool("updater.enable_notification") }()
+		go func(i int) { defer wg.Done(); _ = c.Set("directories.data", fmt.Sprintf("/p/%d", i)) }(i)
+		go func() { defer wg.Done(); _, _ = yaml.Marshal(c) }()
+	}
+	wg.Wait()
+}
+
 func TestSchema(t *testing.T) {
 	c := configmap.New()
 	c.SetKeyTypeSchema("string", "")

internal/go-configmap/json.go

@@ -18,7 +18,9 @@ package configmap
 import "encoding/json"
 
 func (c Map) MarshalJSON() ([]byte, error) {
-	return json.Marshal(c.values)
+	c.mux.RLock()
+	defer c.mux.RUnlock()
+	return json.Marshal(deepSnapshot(c.values))
 }
 
 func (c *Map) UnmarshalJSON(data []byte) error {
@@ -27,9 +29,12 @@ func (c *Map) UnmarshalJSON(data []byte) error {
 		return err
 	}
 
+	c.ensureMux()
+	c.mux.Lock()
+	defer c.mux.Unlock()
 	c.values = map[string]any{}
 	for k, v := range flattenMap(in) {
-		if err := c.Set(k, v); err != nil {
+		if err := c.setValue(k, v); err != nil {
 			return err
 		}
 	}

internal/go-configmap/yaml.go

@@ -20,7 +20,9 @@ import (
 )
 
 func (c Map) MarshalYAML() (any, error) {
-	return c.values, nil
+	c.mux.RLock()
+	defer c.mux.RUnlock()
+	return deepSnapshot(c.values), nil
 }
 
 func (c *Map) UnmarshalYAML(node *yaml.Node) error {
@@ -29,9 +31,12 @@ func (c *Map) UnmarshalYAML(node *yaml.Node) error {
 		return err
 	}
 
+	c.ensureMux()
+	c.mux.Lock()
+	defer c.mux.Unlock()
 	errs := &UnmarshalErrors{}
 	for k, v := range flattenMap(in) {
-		if err := c.Set(k, v); err != nil {
+		if err := c.setValue(k, v); err != nil {
 			errs.append(err)
 		}
 	}