diff --git a/pkg/yqlib/lib.go b/pkg/yqlib/lib.go index 433a5572..1503572b 100644 --- a/pkg/yqlib/lib.go +++ b/pkg/yqlib/lib.go @@ -8,14 +8,24 @@ import ( "math" "strconv" "strings" + "sync" ) var ExpressionParser ExpressionParserInterface +var expressionParserOnce sync.Once + +// InitExpressionParser initialises the package level ExpressionParser, and is +// safe to call concurrently. The evaluators call it on every Evaluate, so +// without the guard two goroutines could construct a parser at the same time, +// and newParticipleLexer populates the shared participleYqRules entries as it +// goes, which a third goroutine could be reading through getYqDefinition. func InitExpressionParser() { - if ExpressionParser == nil { - ExpressionParser = newExpressionParser() - } + expressionParserOnce.Do(func() { + if ExpressionParser == nil { + ExpressionParser = newExpressionParser() + } + }) } var log = newLogger() diff --git a/pkg/yqlib/lib_test.go b/pkg/yqlib/lib_test.go index f35ebb4f..4e031dfd 100644 --- a/pkg/yqlib/lib_test.go +++ b/pkg/yqlib/lib_test.go @@ -3,6 +3,7 @@ package yqlib import ( "fmt" "strings" + "sync" "testing" "github.com/mikefarah/yq/v4/test" @@ -555,3 +556,39 @@ func TestProcessEscapeCharacters(t *testing.T) { test.AssertResultComplexWithContext(t, tt.expected, actual, fmt.Sprintf("Input: %q", tt.input)) } } + +// TestInitExpressionParserConcurrent covers the data race described in #2788. +// The evaluators call InitExpressionParser on every Evaluate, so before it was +// guarded two goroutines could build a parser at the same time while a third +// read the shared participleYqRules entries that newParticipleLexer fills in. +// +// To reproduce the original race the process must not have initialised the +// parser yet, so run this test on its own with the detector enabled: +// +// go test -race -run TestInitExpressionParserConcurrent ./pkg/yqlib/ +func TestInitExpressionParserConcurrent(t *testing.T) { + const goroutines = 32 + + var wg sync.WaitGroup + errs := make(chan error, goroutines) + + for range goroutines { + wg.Add(1) + go func() { + defer wg.Done() + InitExpressionParser() + if _, err := ExpressionParser.ParseExpression(".a.b"); err != nil { + errs <- err + } + }() + } + wg.Wait() + close(errs) + + for err := range errs { + t.Fatalf("concurrent ParseExpression failed: %v", err) + } + if ExpressionParser == nil { + t.Fatal("ExpressionParser should be initialised after concurrent InitExpressionParser calls") + } +}