lang: ast: Fix data race in ExprSingleton

Init the mutex everywhere, but consider calling Init instead and
plumbing though the data input field in the future.
This commit is contained in:
Samuel Gélineau
2024-01-02 22:12:55 -05:00
committed by James Shubin
parent c4a9560d53
commit 3553eb1f2a
3 changed files with 17 additions and 0 deletions

View File

@@ -26,6 +26,7 @@ import (
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
"sync"
"github.com/purpleidea/mgmt/engine" "github.com/purpleidea/mgmt/engine"
engineUtil "github.com/purpleidea/mgmt/engine/util" engineUtil "github.com/purpleidea/mgmt/engine/util"
@@ -3560,6 +3561,8 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error {
newScope.Variables[bind.Ident] = &ExprTopLevel{ newScope.Variables[bind.Ident] = &ExprTopLevel{
Definition: &ExprSingleton{ Definition: &ExprSingleton{
Definition: bind.Value, Definition: bind.Value,
mutex: &sync.Mutex{}, // TODO: call Init instead
}, },
CapturedScope: newScope, CapturedScope: newScope,
} }
@@ -4510,6 +4513,8 @@ func (obj *StmtInclude) SetScope(scope *interfaces.Scope) error {
newScope.Variables[arg.Name] = &ExprTopLevel{ newScope.Variables[arg.Name] = &ExprTopLevel{
Definition: &ExprSingleton{ Definition: &ExprSingleton{
Definition: obj.Args[i], Definition: obj.Args[i],
mutex: &sync.Mutex{}, // TODO: call Init instead
}, },
CapturedScope: newScope, CapturedScope: newScope,
} }
@@ -8943,6 +8948,7 @@ type ExprSingleton struct {
singletonGraph *pgraph.Graph singletonGraph *pgraph.Graph
singletonExpr interfaces.Func singletonExpr interfaces.Func
mutex *sync.Mutex // protects singletonGraph and singletonExpr
} }
// String returns a short representation of this expression. // String returns a short representation of this expression.
@@ -8965,6 +8971,7 @@ func (obj *ExprSingleton) Apply(fn func(interfaces.Node) error) error {
// Init initializes this branch of the AST, and returns an error if it fails to // Init initializes this branch of the AST, and returns an error if it fails to
// validate. // validate.
func (obj *ExprSingleton) Init(data *interfaces.Data) error { func (obj *ExprSingleton) Init(data *interfaces.Data) error {
obj.mutex = &sync.Mutex{}
return obj.Definition.Init(data) return obj.Definition.Init(data)
} }
@@ -8981,6 +8988,7 @@ func (obj *ExprSingleton) Interpolate() (interfaces.Expr, error) {
Definition: definition, Definition: definition,
singletonGraph: nil, // each copy should have its own Graph singletonGraph: nil, // each copy should have its own Graph
singletonExpr: nil, // each copy should have its own Func singletonExpr: nil, // each copy should have its own Func
mutex: &sync.Mutex{},
}, nil }, nil
} }
@@ -8995,6 +9003,7 @@ func (obj *ExprSingleton) Copy() (interfaces.Expr, error) {
Definition: definition, Definition: definition,
singletonGraph: nil, // each copy should have its own Graph singletonGraph: nil, // each copy should have its own Graph
singletonExpr: nil, // each copy should have its own Func singletonExpr: nil, // each copy should have its own Func
mutex: &sync.Mutex{},
}, nil }, nil
} }
@@ -9093,6 +9102,9 @@ func (obj *ExprSingleton) Unify() ([]interfaces.Invariant, error) {
// that fulfill the Stmt interface do not produces vertices, where as their // that fulfill the Stmt interface do not produces vertices, where as their
// children might. // children might.
func (obj *ExprSingleton) Graph(env map[string]interfaces.Func) (*pgraph.Graph, interfaces.Func, error) { func (obj *ExprSingleton) Graph(env map[string]interfaces.Func) (*pgraph.Graph, interfaces.Func, error) {
obj.mutex.Lock()
defer obj.mutex.Unlock()
if obj.singletonExpr == nil { if obj.singletonExpr == nil {
g, f, err := obj.Definition.Graph(env) g, f, err := obj.Definition.Graph(env)
if err != nil { if err != nil {

View File

@@ -20,6 +20,7 @@ package ast
import ( import (
"fmt" "fmt"
"strings" "strings"
"sync"
"github.com/purpleidea/mgmt/lang/funcs" "github.com/purpleidea/mgmt/lang/funcs"
"github.com/purpleidea/mgmt/lang/funcs/simple" "github.com/purpleidea/mgmt/lang/funcs/simple"
@@ -90,6 +91,7 @@ func FuncPrefixToFunctionsScope(prefix string) map[string]interfaces.Expr {
// VarPrefixToVariablesScope is a helper function to return the variables // VarPrefixToVariablesScope is a helper function to return the variables
// portion of the scope from a variable prefix lookup. Basically this is useful // portion of the scope from a variable prefix lookup. Basically this is useful
// to pull out a portion of the variables we've defined by API. // to pull out a portion of the variables we've defined by API.
// TODO: pass `data` into here so we can plumb it into Init for Expr's ?
func VarPrefixToVariablesScope(prefix string) map[string]interfaces.Expr { func VarPrefixToVariablesScope(prefix string) map[string]interfaces.Expr {
fns := vars.LookupPrefix(prefix) // map[string]func() interfaces.Var fns := vars.LookupPrefix(prefix) // map[string]func() interfaces.Var
exprs := make(map[string]interfaces.Expr) exprs := make(map[string]interfaces.Expr)
@@ -102,6 +104,8 @@ func VarPrefixToVariablesScope(prefix string) map[string]interfaces.Expr {
exprs[name] = &ExprTopLevel{ exprs[name] = &ExprTopLevel{
Definition: &ExprSingleton{ Definition: &ExprSingleton{
Definition: expr, Definition: expr,
mutex: &sync.Mutex{}, // TODO: call Init instead
}, },
CapturedScope: interfaces.EmptyScope(), CapturedScope: interfaces.EmptyScope(),
} }

View File

@@ -169,6 +169,7 @@ func (obj *Lang) Init() error {
// TODO: change to a func when we can change hostname dynamically! // TODO: change to a func when we can change hostname dynamically!
"hostname": &ast.ExprStr{V: obj.Hostname}, "hostname": &ast.ExprStr{V: obj.Hostname},
} }
// TODO: pass `data` into ast.VarPrefixToVariablesScope ?
consts := ast.VarPrefixToVariablesScope(vars.ConstNamespace) // strips prefix! consts := ast.VarPrefixToVariablesScope(vars.ConstNamespace) // strips prefix!
addback := vars.ConstNamespace + interfaces.ModuleSep // add it back... addback := vars.ConstNamespace + interfaces.ModuleSep // add it back...
variables, err = ast.MergeExprMaps(variables, consts, addback) variables, err = ast.MergeExprMaps(variables, consts, addback)