Fix race cond in variabels (#523)

* Add race cond test

* Move variables array to vm

* Resize vars array for different programs

Author: antonmedv

.github/workflows/test.yml

@@ -31,3 +31,14 @@ jobs:
           go-version: 1.18
       - name: Test
         run: go test -tags=expr_debug -run=TestDebugger -v ./vm
+
+  race:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Setup Go 1.21
+        uses: actions/setup-go@v4
+        with:
+          go-version: 1.21
+      - name: Test
+        run: go test -race .

compiler/compiler.go

@@ -64,7 +64,7 @@ type compiler struct {
 	config         *conf.Config
 	locations      []file.Location
 	bytecode       []Opcode
-	variables      []any
+	variables      int
 	scopes         []scope
 	constants      []any
 	constantsIndex map[any]int
@@ -137,10 +137,9 @@ func (c *compiler) addConstant(constant any) int {
 }
 
 func (c *compiler) addVariable(name string) int {
-	c.variables = append(c.variables, nil)
-	p := len(c.variables) - 1
-	c.debugInfo[fmt.Sprintf("var_%d", p)] = name
-	return p
+	c.variables++
+	c.debugInfo[fmt.Sprintf("var_%d", c.variables-1)] = name
+	return c.variables - 1
 }
 
 // emitFunction adds builtin.Function.Func to the program.functions and emits call opcode.

expr_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"reflect"
+	"sync"
 	"testing"
 	"time"
 
@@ -2383,3 +2384,22 @@ func TestIssue474(t *testing.T) {
 		})
 	}
 }
+
+func TestRaceCondition_variables(t *testing.T) {
+	program, err := expr.Compile(`let foo = 1; foo + 1`, expr.Env(mock.Env{}))
+	require.NoError(t, err)
+
+	var wg sync.WaitGroup
+
+	for i := 0; i < 10; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			out, err := expr.Run(program, mock.Env{})
+			require.NoError(t, err)
+			require.Equal(t, 2, out)
+		}()
+	}
+
+	wg.Wait()
+}

vm/program.go

@@ -22,7 +22,7 @@ type Program struct {
 
 	source    *file.Source
 	locations []file.Location
-	variables []any
+	variables int
 	functions []Function
 	debugInfo map[string]string
 }
@@ -31,7 +31,7 @@ type Program struct {
 func NewProgram(
 	source *file.Source,
 	locations []file.Location,
-	variables []any,
+	variables int,
 	constants []any,
 	bytecode []Opcode,
 	arguments []int,

vm/vm.go

@@ -34,6 +34,7 @@ func Debug() *VM {
 type VM struct {
 	Stack        []any
 	Scopes       []*Scope
+	Variables    []any
 	ip           int
 	memory       uint
 	memoryBudget uint
@@ -74,10 +75,12 @@ func (vm *VM) Run(program *Program, env any) (_ any, err error) {
 	} else {
 		vm.Stack = vm.Stack[0:0]
 	}
-
 	if vm.Scopes != nil {
 		vm.Scopes = vm.Scopes[0:0]
 	}
+	if len(vm.Variables) < program.variables {
+		vm.Variables = make([]any, program.variables)
+	}
 
 	vm.memoryBudget = MemoryBudget
 	vm.memory = 0
@@ -107,10 +110,10 @@ func (vm *VM) Run(program *Program, env any) (_ any, err error) {
 			vm.pop()
 
 		case OpStore:
-			program.variables[arg] = vm.pop()
+			vm.Variables[arg] = vm.pop()
 
 		case OpLoadVar:
-			vm.push(program.variables[arg])
+			vm.push(vm.Variables[arg])
 
 		case OpLoadConst:
 			vm.push(runtime.Fetch(env, program.Constants[arg]))

vm/vm_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/expr-lang/expr"
 	"github.com/expr-lang/expr/checker"
 	"github.com/expr-lang/expr/compiler"
 	"github.com/expr-lang/expr/conf"
@@ -34,6 +35,28 @@ func TestRun_ReuseVM(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestRun_ReuseVM_for_different_variables(t *testing.T) {
+	v := vm.VM{}
+
+	program, err := expr.Compile(`let a = 1; a + 1`)
+	require.NoError(t, err)
+	out, err := v.Run(program, nil)
+	require.NoError(t, err)
+	require.Equal(t, 2, out)
+
+	program, err = expr.Compile(`let a = 2; a + 1`)
+	require.NoError(t, err)
+	out, err = v.Run(program, nil)
+	require.NoError(t, err)
+	require.Equal(t, 3, out)
+
+	program, err = expr.Compile(`let a = 2; let b = 2; a + b`)
+	require.NoError(t, err)
+	out, err = v.Run(program, nil)
+	require.NoError(t, err)
+	require.Equal(t, 4, out)
+}
+
 func TestRun_Cast(t *testing.T) {
 	input := `1`