fix ordered map race

internal/omap/orderedmap.go

@@ -4,6 +4,7 @@ import (
  "cmp"
  "fmt"
  "slices"
+ "sync"
 
  "gopkg.in/yaml.v3"
 
@@ -15,61 +16,76 @@ import (
 // list of the map's keys. This allows you to run deterministic and ordered
 // operations on the map such as printing/serializing/iterating.
 type OrderedMap[K cmp.Ordered, V any] struct {
- s []K
- m map[K]V
+ mutex sync.RWMutex
+ s []K
+ m map[K]V
 }
 
 // New will create a new OrderedMap of the given type and return it.
 func New[K cmp.Ordered, V any]() OrderedMap[K, V] {
- return OrderedMap[K, V]{
- s: make([]K, 0),
- m: make(map[K]V),
- }
+ return OrderedMap[K, V]{}
 }
 
 // FromMap will create a new OrderedMap from the given map. Since Golang maps
 // are unordered, the order of the created OrderedMap will be random.
 func FromMap[K cmp.Ordered, V any](m map[K]V) OrderedMap[K, V] {
- om := New[K, V]()
- om.m = m
- om.s = exp.Keys(m)
- return om
+ mm := deepcopy.Map(m)
+ ms := exp.Keys(mm)
+ return OrderedMap[K, V]{
+ m: mm,
+ s: ms,
+ }
 }
 
 func FromMapWithOrder[K cmp.Ordered, V any](m map[K]V, order []K) OrderedMap[K, V] {
- om := New[K, V]()
  if len(m) != len(order) {
  panic("length of map and order must be equal")
  }
- om.m = m
- om.s = order
- for key := range om.m {
- if !slices.Contains(om.s, key) {
+
+ for key := range m {
+ if !slices.Contains(order, key) {
  panic("order keys must match map keys")
  }
  }
- return om
+
+ return OrderedMap[K, V]{
+ s: slices.Clone(order),
+ m: deepcopy.Map(m),
+ }
 }
 
 // Len will return the number of items in the map.
-func (om *OrderedMap[K, V]) Len() int {
- return len(om.s)
+func (om *OrderedMap[K, V]) Len() (l int) {
+ om.mutex.RLock()
+ l = len(om.s)
+ om.mutex.RUnlock()
+
+ return
 }
 
 // Set will set the value for a given key.
 func (om *OrderedMap[K, V]) Set(key K, value V) {
+ om.mutex.Lock()
  if om.m == nil {
  om.m = make(map[K]V)
  }
  if _, ok := om.m[key]; !ok {
  om.s = append(om.s, key)
  }
  om.m[key] = value
+ om.mutex.Unlock()
 }
 
 // Get will return the value for a given key.
 // If the key does not exist, it will return the zero value of the value type.
 func (om *OrderedMap[K, V]) Get(key K) V {
+ om.mutex.RLock()
+ defer om.mutex.RUnlock()
+
+ if om.m == nil {
+ var zero V
+ return zero
+ }
  value, ok := om.m[key]
  if !ok {
  var zero V
@@ -80,61 +96,86 @@ func (om *OrderedMap[K, V]) Get(key K) V {
 
 // Exists will return whether or not the given key exists.
 func (om *OrderedMap[K, V]) Exists(key K) bool {
+ om.mutex.RLock()
  _, ok := om.m[key]
+ om.mutex.RUnlock()
  return ok
 }
 
 // Sort will sort the map.
 func (om *OrderedMap[K, V]) Sort() {
+ om.mutex.Lock()
  slices.Sort(om.s)
+ om.mutex.Unlock()
 }
 
 // SortFunc will sort the map using the given function.
 func (om *OrderedMap[K, V]) SortFunc(less func(i, j K) int) {
+ om.mutex.Lock()
  slices.SortFunc(om.s, less)
+ om.mutex.Unlock()
 }
 
 // Keys will return a slice of the map's keys in order.
 func (om *OrderedMap[K, V]) Keys() []K {
- return om.s
+ om.mutex.RLock()
+ keys := deepcopy.Slice(om.s)
+ om.mutex.RUnlock()
+
+ return keys
 }
 
 // Values will return a slice of the map's values in order.
 func (om *OrderedMap[K, V]) Values() []V {
- var values []V
+ om.mutex.RLock()
+ values := make([]V, 0, len(om.m))
  for _, key := range om.s {
  values = append(values, om.m[key])
  }
+ om.mutex.RUnlock()
+
  return values
 }
 
 // Range will iterate over the map and call the given function for each key/value.
 func (om *OrderedMap[K, V]) Range(fn func(key K, value V) error) error {
- for _, key := range om.s {
- if err := fn(key, om.m[key]); err != nil {
+ keys := om.Keys()
+ for _, key := range keys {
+ if err := fn(key, om.Get(key)); err != nil {
  return err
  }
  }
  return nil
 }
 
 // Merge merges the given Vars into the caller one
-func (om *OrderedMap[K, V]) Merge(other OrderedMap[K, V]) {
- // nolint: errcheck
- other.Range(func(key K, value V) error {
+func (om *OrderedMap[K, V]) Merge(other *OrderedMap[K, V]) {
+ _ = other.Range(func(key K, value V) error {
  om.Set(key, value)
  return nil
  })
 }
 
 func (om *OrderedMap[K, V]) DeepCopy() OrderedMap[K, V] {
+ om.mutex.RLock()
+ s := deepcopy.Slice(om.s)
+ m := deepcopy.Map(om.m)
+ om.mutex.RUnlock()
+
  return OrderedMap[K, V]{
- s: deepcopy.Slice(om.s),
- m: deepcopy.Map(om.m),
+ s: s,
+ m: m,
  }
 }
 
 func (om *OrderedMap[K, V]) UnmarshalYAML(node *yaml.Node) error {
+ if om == nil {
+ *om = OrderedMap[K, V]{
+ m: make(map[K]V),
+ s: make([]K, 0),
+ }
+ }
+
  switch node.Kind {
  // Even numbers contain the keys
  // Odd numbers contain the values
@@ -158,7 +199,7 @@ func (om *OrderedMap[K, V]) UnmarshalYAML(node *yaml.Node) error {
  om.Set(k, v)
  }
  return nil
+ default:
+ return fmt.Errorf("yaml: line %d: cannot unmarshal %s into variables", node.Line, node.ShortTag())
  }
-
- return fmt.Errorf("yaml: line %d: cannot unmarshal %s into variables", node.Line, node.ShortTag())
 }

internal/omap/orderedmap_test.go

@@ -89,7 +89,7 @@ func TestOrderedMapMerge(t *testing.T) {
  om2.Set("b", 3)
  om2.Set("c", 4)
 
- om1.Merge(om2)
+ om1.Merge(&om2)
 
  expectedKeys := []string{"a", "b", "c"}
  expectedValues := []int{1, 3, 4}

internal/summary/summary_test.go

@@ -156,13 +156,13 @@ func TestPrintAllWithSpaces(t *testing.T) {
  t2 := &ast.Task{Task: "t2"}
  t3 := &ast.Task{Task: "t3"}
 
- tasks := ast.Tasks{}
- tasks.Set("t1", t1)
- tasks.Set("t2", t2)
- tasks.Set("t3", t3)
+ var tf ast.Taskfile
+ tf.Tasks.Set("t1", t1)
+ tf.Tasks.Set("t2", t2)
+ tf.Tasks.Set("t3", t3)
 
  summary.PrintTasks(&l,
- &ast.Taskfile{Tasks: tasks},
+ &tf,
  []*ast.Call{{Task: "t1"}, {Task: "t2"}, {Task: "t3"}})
 
  assert.True(t, strings.HasPrefix(buffer.String(), "task: t1"))

taskfile/ast/for.go

@@ -52,7 +52,7 @@ func (f *For) UnmarshalYAML(node *yaml.Node) error {
  if forStruct.Var != "" && forStruct.Matrix.Len() != 0 {
  return errors.NewTaskfileDecodeError(nil, node).WithMessage("cannot use both var and matrix in for")
  }
- f.Matrix = forStruct.Matrix
+ f.Matrix = forStruct.Matrix.DeepCopy()
  f.Var = forStruct.Var
  f.Split = forStruct.Split
  f.As = forStruct.As

taskfile/ast/taskfile.go

@@ -55,7 +55,8 @@ func (t1 *Taskfile) Merge(t2 *Taskfile, include *Include) error {
  }
  t1.Vars.Merge(t2.Vars, include)
  t1.Env.Merge(t2.Env, include)
- return t1.Tasks.Merge(t2.Tasks, include, t1.Vars)
+
+ return t1.Tasks.Merge(&t2.Tasks, include, t1.Vars)
 }
 
 func (tf *Taskfile) UnmarshalYAML(node *yaml.Node) error {
@@ -87,7 +88,7 @@ func (tf *Taskfile) UnmarshalYAML(node *yaml.Node) error {
  tf.Shopt = taskfile.Shopt
  tf.Vars = taskfile.Vars
  tf.Env = taskfile.Env
- tf.Tasks = taskfile.Tasks
+ tf.Tasks = taskfile.Tasks.DeepCopy()
  tf.Silent = taskfile.Silent
  tf.Dotenv = taskfile.Dotenv
  tf.Run = taskfile.Run

taskfile/ast/tasks.go

@@ -17,6 +17,12 @@ type Tasks struct {
  omap.OrderedMap[string, *Task]
 }
 
+func (t *Tasks) DeepCopy() Tasks {
+ return Tasks{
+ OrderedMap: t.OrderedMap.DeepCopy(),
+ }
+}
+
 type MatchingTask struct {
  Task *Task
  Wildcards []string
@@ -47,7 +53,7 @@ func (t *Tasks) FindMatchingTasks(call *Call) []*MatchingTask {
  return matchingTasks
 }
 
-func (t1 *Tasks) Merge(t2 Tasks, include *Include, includedTaskfileVars *Vars) error {
+func (t1 *Tasks) Merge(t2 *Tasks, include *Include, includedTaskfileVars *Vars) error {
  err := t2.Range(func(name string, v *Task) error {
  // We do a deep copy of the task struct here to ensure that no data can
  // be changed elsewhere once the taskfile is merged.
@@ -152,13 +158,10 @@ func (t *Tasks) UnmarshalYAML(node *yaml.Node) error {
  }
  }
  }
- tasks.Set(name, task)
+ t.Set(name, task)
  return nil
  })
 
- *t = Tasks{
- OrderedMap: tasks,
- }
  return nil
  }
 

variables.go

@@ -274,7 +274,7 @@ func itemsFromFor(
  var values []any // The list of values to loop over
  // Get the list from a matrix
  if f.Matrix.Len() != 0 {
- return asAnySlice(product(f.Matrix)), nil, nil
+ return asAnySlice(product(&f.Matrix)), nil, nil
  }
  // Get the list from the explicit for list
  if len(f.List) > 0 {
@@ -329,7 +329,7 @@ func itemsFromFor(
 }
 
 // product generates the cartesian product of the input map of slices.
-func product(inputMap omap.OrderedMap[string, []any]) []map[string]any {
+func product(inputMap *omap.OrderedMap[string, []any]) []map[string]any {
  if inputMap.Len() == 0 {
  return nil
  }