Fix a race condition when evaluating on the root context

.github/workflows/e2e.yml

@@ -33,3 +33,18 @@ jobs:
       run: make e2e
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+  race:
+    name: race
+    # go test -race is slow and only runs on ubuntu
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v4
+      with:
+        submodules: true
+    - name: Set up Go
+      uses: actions/setup-go@v5
+      with:
+        go-version-file: 'go.mod'
+    - name: Run e2e-race
+      run: make e2e-race

Makefile

@@ -16,7 +16,10 @@ install:
 	go install
 
 e2e: prepare install
-	go test -timeout 5m ./integrationtest/...
+	go test -timeout 5m $$(go list ./integrationtest/... | grep -v race)
+
+e2e-race: prepare
+	go test --race --timeout 5m ./integrationtest/race
 
 lint:
 	golangci-lint run ./...

integrationtest/race/eval_locals_on_root_ctx/.tflint.hcl

@@ -0,0 +1,3 @@
+plugin "testing" {
+  enabled = true
+}

integrationtest/race/eval_locals_on_root_ctx/main.tf

@@ -0,0 +1,17 @@
+locals {
+  dns_name = "www.example.com"
+}
+
+resource "aws_route53_record" "www" {
+  zone_id = aws_route53_zone.primary.zone_id
+  name    = local.dns_name
+  type    = "A"
+  ttl     = 300
+  records = [aws_eip.lb.public_ip]
+}
+
+module "route53_records" {
+  count = 10
+
+  source = "./module"
+}

integrationtest/race/eval_locals_on_root_ctx/module/main.tf

@@ -0,0 +1,7 @@
+resource "aws_route53_record" "help" {
+  zone_id = aws_route53_zone.primary.zone_id
+  name    = "help.example.com"
+  type    = "A"
+  ttl     = 300
+  records = [aws_eip.lb.public_ip]
+}

integrationtest/race/eval_locals_on_root_ctx/result.json

@@ -0,0 +1,25 @@
+{
+  "issues": [
+    {
+      "rule": {
+        "name": "aws_route53_record_eval_on_root_ctx_example",
+        "severity": "error",
+        "link": ""
+      },
+      "message": "record name (root): \"www.example.com\"",
+      "range": {
+        "filename": "main.tf",
+        "start": {
+          "line": 7,
+          "column": 13
+        },
+        "end": {
+          "line": 7,
+          "column": 27
+        }
+      },
+      "callers": []
+    }
+  ],
+  "errors": []
+}

integrationtest/race/race_test.go

@@ -0,0 +1,116 @@
+package main
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
+	"log"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+	"text/template"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/terraform-linters/tflint/cmd"
+	"github.com/terraform-linters/tflint/formatter"
+	"github.com/terraform-linters/tflint/tflint"
+)
+
+func TestMain(m *testing.M) {
+	log.SetOutput(io.Discard)
+	os.Exit(m.Run())
+}
+
+type meta struct {
+	Version string
+}
+
+func TestIntegration(t *testing.T) {
+	cases := []struct {
+		Name    string
+		Command string
+		Dir     string
+	}{
+		{
+			// @see https://github.com/terraform-linters/tflint/issues/2094
+			Name:    "eval locals on the root context in parallel runners",
+			Command: "tflint --format json",
+			Dir:     "eval_locals_on_root_ctx",
+		},
+	}
+
+	// Disable the bundled plugin because the `os.Executable()` is go(1) in the tests
+	tflint.DisableBundledPlugin = true
+	defer func() {
+		tflint.DisableBundledPlugin = false
+	}()
+
+	dir, _ := os.Getwd()
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			testDir := filepath.Join(dir, tc.Dir)
+
+			defer func() {
+				if err := os.Chdir(dir); err != nil {
+					t.Fatal(err)
+				}
+			}()
+			if err := os.Chdir(testDir); err != nil {
+				t.Fatal(err)
+			}
+
+			outStream, errStream := new(bytes.Buffer), new(bytes.Buffer)
+			cli, err := cmd.NewCLI(outStream, errStream)
+			if err != nil {
+				t.Fatal(err)
+			}
+			args := strings.Split(tc.Command, " ")
+
+			cli.Run(args)
+
+			rawWant, err := readResultFile(testDir)
+			if err != nil {
+				t.Fatal(err)
+			}
+			var want *formatter.JSONOutput
+			if err := json.Unmarshal(rawWant, &want); err != nil {
+				t.Fatal(err)
+			}
+
+			var got *formatter.JSONOutput
+			if err := json.Unmarshal(outStream.Bytes(), &got); err != nil {
+				t.Fatal(err)
+			}
+
+			if diff := cmp.Diff(got, want); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+	}
+}
+
+func readResultFile(dir string) ([]byte, error) {
+	resultFile := "result.json"
+	if runtime.GOOS == "windows" {
+		if _, err := os.Stat(filepath.Join(dir, "result_windows.json")); !os.IsNotExist(err) {
+			resultFile = "result_windows.json"
+		}
+	}
+	if _, err := os.Stat(fmt.Sprintf("%s.tmpl", resultFile)); !os.IsNotExist(err) {
+		resultFile = fmt.Sprintf("%s.tmpl", resultFile)
+	}
+
+	if !strings.HasSuffix(resultFile, ".tmpl") {
+		return os.ReadFile(filepath.Join(dir, resultFile))
+	}
+
+	want := new(bytes.Buffer)
+	tmpl := template.Must(template.ParseFiles(filepath.Join(dir, resultFile)))
+	if err := tmpl.Execute(want, meta{Version: tflint.Version.String()}); err != nil {
+		return nil, err
+	}
+	return want.Bytes(), nil
+}

terraform/evaluator.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/agext/levenshtein"
 	"github.com/hashicorp/hcl/v2"
@@ -21,64 +20,11 @@ type ContextMeta struct {
 	OriginalWorkingDir string
 }
 
-type CallStack struct {
-	addrs map[string]addrs.Reference
-	stack []string
-}
-
-func NewCallStack() *CallStack {
-	return &CallStack{
-		addrs: make(map[string]addrs.Reference),
-		stack: make([]string, 0),
-	}
-}
-
-func (g *CallStack) Push(addr addrs.Reference) hcl.Diagnostics {
-	g.stack = append(g.stack, addr.Subject.String())
-
-	if _, exists := g.addrs[addr.Subject.String()]; exists {
-		return hcl.Diagnostics{
-			{
-				Severity: hcl.DiagError,
-				Summary:  "circular reference found",
-				Detail:   g.String(),
-				Subject:  addr.SourceRange.Ptr(),
-			},
-		}
-	}
-	g.addrs[addr.Subject.String()] = addr
-	return hcl.Diagnostics{}
-}
-
-func (g *CallStack) Pop() {
-	if g.Empty() {
-		panic("cannot pop from empty stack")
-	}
-
-	addr := g.stack[len(g.stack)-1]
-	g.stack = g.stack[:len(g.stack)-1]
-	delete(g.addrs, addr)
-}
-
-func (g *CallStack) String() string {
-	return strings.Join(g.stack, " -> ")
-}
-
-func (g *CallStack) Empty() bool {
-	return len(g.stack) == 0
-}
-
-func (g *CallStack) Clear() {
-	g.addrs = make(map[string]addrs.Reference)
-	g.stack = make([]string, 0)
-}
-
 type Evaluator struct {
 	Meta           *ContextMeta
 	ModulePath     addrs.ModuleInstance
 	Config         *Config
 	VariableValues map[string]map[string]cty.Value
-	CallStack      *CallStack
 }
 
 // EvaluateExpr takes the given HCL expression and evaluates it to produce a value.
@@ -101,18 +47,27 @@ func (e *Evaluator) ExpandBlock(body hcl.Body, schema *hclext.BodySchema) (hcl.B
 	return e.scope().ExpandBlock(body, schema)
 }
 
+// scope creates a new evaluation scope.
+// The difference with Evaluator is that each evaluation is independent
+// and is not shared between goroutines.
 func (e *Evaluator) scope() *lang.Scope {
-	return &lang.Scope{
-		Data: &evaluationData{
-			Evaluator:  e,
-			ModulePath: e.ModulePath,
-		},
+	scope := &lang.Scope{CallStack: lang.NewCallStack()}
+	scope.Data = &evaluationData{
+		Scope:          scope,
+		Meta:           e.Meta,
+		ModulePath:     e.ModulePath,
+		Config:         e.Config,
+		VariableValues: e.VariableValues,
 	}
+	return scope
 }
 
 type evaluationData struct {
-	Evaluator  *Evaluator
-	ModulePath addrs.ModuleInstance
+	Scope          *lang.Scope
+	Meta           *ContextMeta
+	ModulePath     addrs.ModuleInstance
+	Config         *Config
+	VariableValues map[string]map[string]cty.Value
 }
 
 var _ lang.Data = (*evaluationData)(nil)
@@ -142,7 +97,7 @@ func (d *evaluationData) GetForEachAttr(addr addrs.ForEachAttr, rng hcl.Range) (
 func (d *evaluationData) GetInputVariable(addr addrs.InputVariable, rng hcl.Range) (cty.Value, hcl.Diagnostics) {
 	var diags hcl.Diagnostics
 
-	moduleConfig := d.Evaluator.Config.DescendentForInstance(d.ModulePath)
+	moduleConfig := d.Config.DescendentForInstance(d.ModulePath)
 	if moduleConfig == nil {
 		// should never happen, since we can't be evaluating in a module
 		// that wasn't mentioned in configuration.
@@ -172,7 +127,7 @@ func (d *evaluationData) GetInputVariable(addr addrs.InputVariable, rng hcl.Rang
 	}
 
 	moduleAddrStr := d.ModulePath.String()
-	vals := d.Evaluator.VariableValues[moduleAddrStr]
+	vals := d.VariableValues[moduleAddrStr]
 	if vals == nil {
 		return cty.UnknownVal(config.Type), diags
 	}
@@ -229,7 +184,7 @@ func (d *evaluationData) GetLocalValue(addr addrs.LocalValue, rng hcl.Range) (ct
 
 	// First we'll make sure the requested value is declared in configuration,
 	// so we can produce a nice message if not.
-	moduleConfig := d.Evaluator.Config.DescendentForInstance(d.ModulePath)
+	moduleConfig := d.Config.DescendentForInstance(d.ModulePath)
 	if moduleConfig == nil {
 		// should never happen, since we can't be evaluating in a module
 		// that wasn't mentioned in configuration.
@@ -257,13 +212,13 @@ func (d *evaluationData) GetLocalValue(addr addrs.LocalValue, rng hcl.Range) (ct
 	}
 
 	// Build a call stack for circular reference detection only when getting a local value.
-	if diags := d.Evaluator.CallStack.Push(addrs.Reference{Subject: addr, SourceRange: rng}); diags.HasErrors() {
+	if diags := d.Scope.CallStack.Push(addrs.Reference{Subject: addr, SourceRange: rng}); diags.HasErrors() {
 		return cty.UnknownVal(cty.DynamicPseudoType), diags
 	}
 
-	val, diags := d.Evaluator.EvaluateExpr(config.Expr, cty.DynamicPseudoType)
+	val, diags := d.Scope.EvalExpr(config.Expr, cty.DynamicPseudoType)
 
-	d.Evaluator.CallStack.Pop()
+	d.Scope.CallStack.Pop()
 	return val, diags
 }
 
@@ -274,10 +229,10 @@ func (d *evaluationData) GetPathAttr(addr addrs.PathAttr, rng hcl.Range) (cty.Va
 	case "cwd":
 		var err error
 		var wd string
-		if d.Evaluator.Meta != nil {
+		if d.Meta != nil {
 			// Meta is always non-nil in the normal case, but some test cases
 			// are not so realistic.
-			wd = d.Evaluator.Meta.OriginalWorkingDir
+			wd = d.Meta.OriginalWorkingDir
 		}
 		if wd == "" {
 			wd, err = os.Getwd()
@@ -308,7 +263,7 @@ func (d *evaluationData) GetPathAttr(addr addrs.PathAttr, rng hcl.Range) (cty.Va
 		return cty.StringVal(filepath.ToSlash(wd)), diags
 
 	case "module":
-		moduleConfig := d.Evaluator.Config.DescendentForInstance(d.ModulePath)
+		moduleConfig := d.Config.DescendentForInstance(d.ModulePath)
 		if moduleConfig == nil {
 			// should never happen, since we can't be evaluating in a module
 			// that wasn't mentioned in configuration.
@@ -318,7 +273,7 @@ func (d *evaluationData) GetPathAttr(addr addrs.PathAttr, rng hcl.Range) (cty.Va
 		return cty.StringVal(filepath.ToSlash(sourceDir)), diags
 
 	case "root":
-		sourceDir := d.Evaluator.Config.Module.SourceDir
+		sourceDir := d.Config.Module.SourceDir
 		return cty.StringVal(filepath.ToSlash(sourceDir)), diags
 
 	default:
@@ -341,7 +296,7 @@ func (d *evaluationData) GetTerraformAttr(addr addrs.TerraformAttr, rng hcl.Rang
 	switch addr.Name {
 
 	case "workspace":
-		workspaceName := d.Evaluator.Meta.Env
+		workspaceName := d.Meta.Env
 		return cty.StringVal(workspaceName), diags
 
 	case "env":