From 51221feb3bef9a6f22905db420b55408c56834cc Mon Sep 17 00:00:00 2001 From: Reilly Watson Date: Tue, 16 Sep 2025 13:14:37 -0400 Subject: [PATCH] protect global map access with read lock If you call AddOperator() and either Apply() or IsValid() concurrently, the Go runtime can throw a fatal error about a concurrent map read+write. Similarly, tests that perform these operations concurrently will trigger race detector failures. Make the operators lock into an RWMutex and hold a read lock when accessing the map to avoid this race. --- operation.go | 12 ++++---- operation_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++++++ validator.go | 2 ++ 3 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 operation_test.go diff --git a/operation.go b/operation.go index 7b4e869..7255d14 100644 --- a/operation.go +++ b/operation.go @@ -19,12 +19,12 @@ func (e ErrInvalidOperator) Error() string { // operators holds custom operators var operators = make(map[string]OperatorFn) -var operatorsWriteLock = &sync.Mutex{} +var operatorsLock = &sync.RWMutex{} // AddOperator allows for custom operators to be used func AddOperator(key string, cb OperatorFn) { - operatorsWriteLock.Lock() - defer operatorsWriteLock.Unlock() + operatorsLock.Lock() + defer operatorsLock.Unlock() operators[key] = func(values, data any) any { return cb(parseValues(values, data), data) @@ -32,7 +32,9 @@ func AddOperator(key string, cb OperatorFn) { } func operation(operator string, values, data any) any { + operatorsLock.RLock() opFn, found := operators[operator] + operatorsLock.RUnlock() if found { return opFn(values, data) } @@ -43,8 +45,8 @@ func operation(operator string, values, data any) any { } func init() { - operatorsWriteLock.Lock() - defer operatorsWriteLock.Unlock() + operatorsLock.Lock() + defer operatorsLock.Unlock() operators["and"] = _and operators["or"] = _or diff --git a/operation_test.go b/operation_test.go new file mode 100644 index 0000000..ac1ad58 --- /dev/null +++ b/operation_test.go @@ -0,0 +1,75 @@ +package jsonlogic + +import ( + "io" + "strings" + "sync" + "testing" +) + +// TestConcurrentApplyAndAddOperator validates that validating rules and adding operators concurrently +// doesn't cause fatal errors or deadlocks. +func TestConcurrentValidationAndAddOperator(t *testing.T) { + var wg sync.WaitGroup + numRoutines := 10 + numIterations := 100 + + // Start multiple goroutines to validate rules concurrently + for i := 0; i < numRoutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < numIterations; j++ { + rule := `{"==": [1, 1]}` + _ = IsValid(strings.NewReader(rule)) + } + }() + } + + // Start a goroutine to add a new operator concurrently + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < numIterations; j++ { + AddOperator("test_op", func(values, data any) any { + return "test" + }) + } + }() + + wg.Wait() +} + +// TestConcurrentApplyAndAddOperator validates that applying rules and adding operators concurrently +// doesn't cause fatal errors or deadlocks. +func TestConcurrentApplyAndAddOperator(t *testing.T) { + var wg sync.WaitGroup + numRoutines := 10 + numIterations := 100 + + // Start multiple goroutines to apply rules concurrently + for i := 0; i < numRoutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < numIterations; j++ { + rule := `{"==": [1, 1]}` + data := `{}` + _ = Apply(strings.NewReader(rule), strings.NewReader(data), io.Discard) + } + }() + } + + // Start a goroutine to add a new operator concurrently + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < numIterations; j++ { + AddOperator("test_op", func(values, data any) any { + return "test" + }) + } + }() + + wg.Wait() +} diff --git a/validator.go b/validator.go index c0237ee..068edc2 100644 --- a/validator.go +++ b/validator.go @@ -64,7 +64,9 @@ func ValidateJsonLogic(rules any) bool { } func isOperator(op string) bool { + operatorsLock.RLock() _, isOperator := operators[op] + operatorsLock.RUnlock() return isOperator }