Fix locking around Fill and Lookup

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>

dagger/compiler.go

@@ -18,7 +18,7 @@ import (
 // (we call it compiler to avoid confusion with dagger runtime)
 // Use this instead of cue.Runtime
 type Compiler struct {
-	sync.Mutex
+	sync.RWMutex
 	cue.Runtime
 	spec *Spec
 }

dagger/env.go

@@ -3,7 +3,6 @@ package dagger
 import (
 	"context"
 	"os"
-	"sync"
 
 	"cuelang.org/go/cue"
 	cueflow "cuelang.org/go/tools/flow"
@@ -144,14 +143,9 @@ func (env *Env) Walk(ctx context.Context, fn EnvWalkFunc) (*Value, error) {
 		return nil, err
 	}
 
-	l := sync.Mutex{}
-
 	// Cueflow config
 	flowCfg := &cueflow.Config{
 		UpdateFunc: func(c *cueflow.Controller, t *cueflow.Task) error {
-			l.Lock()
-			defer l.Unlock()
-
 			if t == nil {
 				return nil
 			}
@@ -180,9 +174,6 @@ func (env *Env) Walk(ctx context.Context, fn EnvWalkFunc) (*Value, error) {
 	}
 	// Cueflow match func
 	flowMatchFn := func(v cue.Value) (cueflow.Runner, error) {
-		l.Lock()
-		defer l.Unlock()
-
 		lg := lg.
 			With().
 			Str("path", v.Path().String()).
@@ -200,9 +191,6 @@ func (env *Env) Walk(ctx context.Context, fn EnvWalkFunc) (*Value, error) {
 			return nil, err
 		}
 		return cueflow.RunnerFunc(func(t *cueflow.Task) error {
-			l.Lock()
-			defer l.Unlock()
-
 			return fn(ctx, c, t)
 		}), nil
 	}

dagger/spec.go

@@ -1,7 +1,6 @@
 package dagger
 
 import (
-	"cuelang.org/go/cue"
 	cueerrors "cuelang.org/go/cue/errors"
 	"github.com/pkg/errors"
 )
@@ -12,28 +11,14 @@ type Spec struct {
 }
 
 // eg. Validate(op, "#Op")
-func (s Spec) Validate(v *Value, defpath string) (err error) {
-	// Expand cue errors to get full details
-	// FIXME: there is probably a cleaner way to do this.
-	defer func() {
-		if err != nil {
-			err = errors.New(cueerrors.Details(err, nil))
-		}
-	}()
-
+func (s Spec) Validate(v *Value, defpath string) error {
 	// Lookup def by name, eg. "#Script" or "#Copy"
 	// See dagger/spec.cue
 	def := s.root.Get(defpath)
-	if err := def.Validate(); err != nil {
-		return err
-	}
-	merged := def.Unwrap().Fill(v.Value)
-	if err := merged.Err(); err != nil {
-		return err
-	}
-	if err := merged.Validate(cue.Final()); err != nil {
-		return err
+	if err := def.Fill(v.Value); err != nil {
+		return errors.New(cueerrors.Details(err, nil))
 	}
+
 	return nil
 }
 

dagger/value.go

@@ -18,31 +18,18 @@ type Value struct {
 	inst *cue.Instance
 }
 
-func (v *Value) Lock() {
-	if v.cc == nil {
-		return
-	}
-	v.cc.Lock()
-}
-
-func (v *Value) Unlock() {
-	if v.cc == nil {
-		return
-	}
-	v.cc.Unlock()
-}
-
 func (v *Value) Lookup(path ...string) *Value {
-	v.Lock()
-	defer v.Unlock()
+	v.cc.RLock()
+	defer v.cc.RUnlock()
 
-	return v.Wrap(v.Unwrap().LookupPath(cueStringsToCuePath(path...)))
+	return v.Wrap(v.Value.LookupPath(cueStringsToCuePath(path...)))
 }
 
 func (v *Value) LookupPath(p cue.Path) *Value {
-	v.Lock()
-	defer v.Unlock()
-	return v.Wrap(v.Unwrap().LookupPath(p))
+	v.cc.RLock()
+	defer v.cc.RUnlock()
+
+	return v.Wrap(v.Value.LookupPath(p))
 }
 
 // FIXME: deprecated by Get()
@@ -150,9 +137,13 @@ func (v *Value) Merge(x interface{}, path ...string) (*Value, error) {
 		if xval.Compiler() != v.Compiler() {
 			return nil, fmt.Errorf("can't merge values from different compilers")
 		}
-		x = xval.Unwrap()
+		x = xval.Value
 	}
-	result := v.Wrap(v.Unwrap().Fill(x, path...))
+
+	v.cc.Lock()
+	result := v.Wrap(v.Value.Fill(x, path...))
+	v.cc.Unlock()
+
 	return result, result.Validate()
 }
 
@@ -226,8 +217,6 @@ func (v *Value) IsConcreteR() bool {
 // Contrast with cue.Value.MarshalJSON which requires all values
 // to be concrete.
 func (v *Value) JSON() JSON {
-	v.Lock()
-	defer v.Unlock()
 	var out JSON
 	v.Walk(
 		func(v cue.Value) bool {
@@ -267,7 +256,7 @@ func (v *Value) Save(fs FS, filename string) (FS, error) {
 }
 
 func (v *Value) Validate(defs ...string) error {
-	if err := v.Unwrap().Validate(); err != nil {
+	if err := v.Value.Validate(); err != nil {
 		return err
 	}
 	if len(defs) == 0 {
@@ -289,13 +278,14 @@ func (v *Value) Validate(defs ...string) error {
 // This is the only method which changes the value in-place.
 // FIXME this co-exists awkwardly with the rest of Value.
 func (v *Value) Fill(x interface{}) error {
+	v.cc.Lock()
+	defer v.cc.Unlock()
+
 	v.Value = v.Value.Fill(x)
 	return v.Validate()
 }
 
 func (v *Value) Source() ([]byte, error) {
-	v.Lock()
-	defer v.Unlock()
 	return cueformat.Node(v.Eval().Syntax())
 }
 
@@ -313,9 +303,6 @@ func (v *Value) CueInst() *cue.Instance {
 }
 
 func (v *Value) Compiler() *Compiler {
-	//	if v.cc == nil {
-	//		return &Compiler{}
-	//	}
 	return v.cc
 }
 
@@ -323,10 +310,6 @@ func (v *Value) Wrap(v2 cue.Value) *Value {
 	return wrapValue(v2, v.inst, v.cc)
 }
 
-func (v *Value) Unwrap() cue.Value {
-	return v.Value
-}
-
 func wrapValue(v cue.Value, inst *cue.Instance, cc *Compiler) *Value {
 	return &Value{
 		Value: v,