From 3553eb1f2a9a11c0341a4f6c10255c14bd74658b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Samuel=20G=C3=A9lineau?= <gelisam+github@gmail.com>
Date: Tue, 2 Jan 2024 22:12:55 -0500
Subject: [PATCH] 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.
---
 lang/ast/structs.go | 12 ++++++++++++
 lang/ast/util.go    |  4 ++++
 lang/lang.go        |  1 +
 3 files changed, 17 insertions(+)

diff --git a/lang/ast/structs.go b/lang/ast/structs.go
index f42b341b..5ee4e481 100644
--- a/lang/ast/structs.go
+++ b/lang/ast/structs.go
@@ -26,6 +26,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/purpleidea/mgmt/engine"
 	engineUtil "github.com/purpleidea/mgmt/engine/util"
@@ -3560,6 +3561,8 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error {
 		newScope.Variables[bind.Ident] = &ExprTopLevel{
 			Definition: &ExprSingleton{
 				Definition: bind.Value,
+
+				mutex: &sync.Mutex{}, // TODO: call Init instead
 			},
 			CapturedScope: newScope,
 		}
@@ -4510,6 +4513,8 @@ func (obj *StmtInclude) SetScope(scope *interfaces.Scope) error {
 		newScope.Variables[arg.Name] = &ExprTopLevel{
 			Definition: &ExprSingleton{
 				Definition: obj.Args[i],
+
+				mutex: &sync.Mutex{}, // TODO: call Init instead
 			},
 			CapturedScope: newScope,
 		}
@@ -8943,6 +8948,7 @@ type ExprSingleton struct {
 
 	singletonGraph *pgraph.Graph
 	singletonExpr  interfaces.Func
+	mutex          *sync.Mutex // protects singletonGraph and singletonExpr
 }
 
 // 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
 // validate.
 func (obj *ExprSingleton) Init(data *interfaces.Data) error {
+	obj.mutex = &sync.Mutex{}
 	return obj.Definition.Init(data)
 }
 
@@ -8981,6 +8988,7 @@ func (obj *ExprSingleton) Interpolate() (interfaces.Expr, error) {
 		Definition:     definition,
 		singletonGraph: nil, // each copy should have its own Graph
 		singletonExpr:  nil, // each copy should have its own Func
+		mutex:          &sync.Mutex{},
 	}, nil
 }
 
@@ -8995,6 +9003,7 @@ func (obj *ExprSingleton) Copy() (interfaces.Expr, error) {
 		Definition:     definition,
 		singletonGraph: nil, // each copy should have its own Graph
 		singletonExpr:  nil, // each copy should have its own Func
+		mutex:          &sync.Mutex{},
 	}, nil
 }
 
@@ -9093,6 +9102,9 @@ func (obj *ExprSingleton) Unify() ([]interfaces.Invariant, error) {
 // that fulfill the Stmt interface do not produces vertices, where as their
 // children might.
 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 {
 		g, f, err := obj.Definition.Graph(env)
 		if err != nil {
diff --git a/lang/ast/util.go b/lang/ast/util.go
index f35956f0..5cc9ce99 100644
--- a/lang/ast/util.go
+++ b/lang/ast/util.go
@@ -20,6 +20,7 @@ package ast
 import (
 	"fmt"
 	"strings"
+	"sync"
 
 	"github.com/purpleidea/mgmt/lang/funcs"
 	"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
 // 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.
+// TODO: pass `data` into here so we can plumb it into Init for Expr's ?
 func VarPrefixToVariablesScope(prefix string) map[string]interfaces.Expr {
 	fns := vars.LookupPrefix(prefix) // map[string]func() interfaces.Var
 	exprs := make(map[string]interfaces.Expr)
@@ -102,6 +104,8 @@ func VarPrefixToVariablesScope(prefix string) map[string]interfaces.Expr {
 		exprs[name] = &ExprTopLevel{
 			Definition: &ExprSingleton{
 				Definition: expr,
+
+				mutex: &sync.Mutex{}, // TODO: call Init instead
 			},
 			CapturedScope: interfaces.EmptyScope(),
 		}
diff --git a/lang/lang.go b/lang/lang.go
index 3709b86d..8754d37f 100644
--- a/lang/lang.go
+++ b/lang/lang.go
@@ -169,6 +169,7 @@ func (obj *Lang) Init() error {
 		// TODO: change to a func when we can change hostname dynamically!
 		"hostname": &ast.ExprStr{V: obj.Hostname},
 	}
+	// TODO: pass `data` into ast.VarPrefixToVariablesScope ?
 	consts := ast.VarPrefixToVariablesScope(vars.ConstNamespace) // strips prefix!
 	addback := vars.ConstNamespace + interfaces.ModuleSep        // add it back...
 	variables, err = ast.MergeExprMaps(variables, consts, addback)