Conversation
The correlated-subquery create closure calls back into the Builder to compile a fresh Subquery instance on demand at runtime. When a recursive function containing a subquery runs in multiple fork branches, those branches race on the Builder's maps (compiledVamUDFs), which can leave a UDF with a nil Body and crash the process. Guard the runtime create path with a mutex. Fixes brimdata#7284
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7284
Problem
The repro in #7284 (a recursive function whose body is a subquery, called from four
forkbranches) segfaults:Building with
-raceshows the cause: the correlated-subquerycreateclosure incompileVamSubquerycalls back into theBuilderat runtime (fromSubquery.Evalwhen the recursion re-enters an instance that's still evaluating). With several fork branches doing this concurrently, they race onBuilder.compiledVamUDFs(compileVamUDFCallinserts the UDF, compiles its body, then deletes the entry). One goroutine can observe another goroutine's half-builtUDFwith a nilBodyand call through it.Fix
Wrap the runtime-invoked
createcallback in aBuildermutex so on-demand subquery compilation is serialized. Compile-time construction of the first instance is unchanged.Test
Added
runtime/ztests/op/recursive-subquery-fork.yamlwith the issue's query (4 branches ×walk(200)→count()=800).Verification
go build -race ./cmd/super, then the repro run 3× — 0DATA RACEreports, output800, exit 0 (before the fix: 4 races per run plus the crash).go test -short -run 'TestSPQ/runtime/ztests/(op|expr)$' .— all pass, includingrecursive-subquery,udf,udf-overflow,udf-mutual-recursion.go test -short ./compiler/... ./runtime/vam/...,go vet ./compiler/rungen/,gofmt -lclean.