From a0c6389b58f53f4a0d9a7a3a26baf292f6667817 Mon Sep 17 00:00:00 2001 From: Michael Pursifull Date: Sun, 2 Aug 2026 21:51:40 -0500 Subject: [PATCH] Guard ExpressionParser initialization with sync.Once (#2789) * Guard ExpressionParser initialization with sync.Once The evaluators call InitExpressionParser on every Evaluate, so two goroutines could construct a parser at the same time. newParticipleLexer also fills in the ParticipleTokenType field on each shared participleYqRules entry as it builds the lexer, which another goroutine could be reading through getYqDefinition. These are pointer and struct field writes rather than map writes, so the runtime does not stop the process; it corrupts quietly instead. The nil check stays inside the Once so a caller that assigns ExpressionParser itself is still respected. Fixes #2788 * docs: use UK spelling for initialised in new comments Matches the existing prose convention in pkg/yqlib (operator_reduce.go, decoder_ini.go, operator_alternative_test.go). Review feedback on #2789. --- pkg/yqlib/lib.go | 16 +++++++++++++--- pkg/yqlib/lib_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) 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") + } +}