Skip to content

Commit 799b897

Browse files
fredbiclaude
andauthored
fix(race): read result validity before Merge redeems it to the pool (#260)
validateRequiredDefinitions read red.IsValid() *after* res.Merge(red). Merge redeems a result whose wantsRedeemOnMerge is set (true for any pool-borrowed result) back into the process-global sync.Pool. Reading red after that point races with a concurrent Spec() goroutine borrowing the same *Result and calling cleared() on it. This surfaced as a rare data race under -race in Test_ParallelPool: Write at ... Result.cleared() <- resultsPool.BorrowResult() Previous read ... Result.IsValid() <- validateRequiredDefinitions Capture validity into a local before merging, matching the read-before- merge pattern used everywhere else (e.g. validateRequiredProperties, default/example validators). Signed-off-by: Frederic BIDON <fredbi@yahoo.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e5b5ca2 commit 799b897

1 file changed

Lines changed: 5 additions & 1 deletion

File tree

spec.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,8 +554,12 @@ DEFINITIONS:
554554
if schema.Required != nil { // Safeguard
555555
for _, pn := range schema.Required {
556556
red := s.validateRequiredProperties(pn, d, &schema) //#nosec
557+
// NOTE: capture validity before merging: Merge may redeem `red` to the
558+
// pool (wantsRedeemOnMerge), after which reading it races with a concurrent
559+
// BorrowResult().cleared() in another goroutine sharing the global pool.
560+
isValid := red.IsValid()
557561
res.Merge(red)
558-
if !red.IsValid() && !s.Options.ContinueOnErrors {
562+
if !isValid && !s.Options.ContinueOnErrors {
559563
break DEFINITIONS // there is an error, let's stop that bleeding
560564
}
561565
}

0 commit comments

Comments
 (0)