this repo has no description
internal/core/adt: fix race condition in Vertex.Default()
When Default() is called on a vertex with a disjunction where
NumDefaults == 1, the recursive call ToVertex(Default(d.Values[0]))
could return a shared vertex. Multiple goroutines calling Default()
on the same value would then race on the check-then-modify pattern:
if w.Conjuncts == nil {
w.Conjuncts = append(w.Conjuncts, ...)
}
Fix by ensuring we always work on a fresh copy before modifying:
- If the recursive result already has conjuncts, return as-is
- Otherwise, make a copy before populating conjuncts
This eliminates the race by ensuring no shared vertex is ever modified.
Updates #2733
Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Iaeb58fe3262d720d3757c5388a91c158cd2615a7
Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1230712
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
+142
-7
+68
-7
internal/core/adt/default.go
+68
-7
internal/core/adt/default.go
···
62
62
return v
63
63
case 1:
64
64
w = ToVertex(Default(d.Values[0]))
65
+
// If w already has conjuncts, return as-is to avoid race.
66
+
if w.Conjuncts != nil {
67
+
return w
68
+
}
69
+
// Make a copy before modifying to avoid racing on shared vertex.
70
+
x := *w
71
+
x.state = nil
72
+
w = &x
65
73
default:
66
74
x := *v
67
75
x.state = nil
···
71
79
NumDefaults: 0,
72
80
}
73
81
w = &x
74
-
w.Conjuncts = nil
75
82
}
76
83
77
-
if w.Conjuncts == nil {
78
-
for _, c := range v.Conjuncts {
79
-
// TODO: preserve field information.
80
-
expr, _ := stripNonDefaults(c.Elem())
81
-
w.Conjuncts = append(w.Conjuncts, MakeRootConjunct(c.Env, expr))
82
-
}
84
+
// w is now a fresh copy, safe to modify without race.
85
+
w.Conjuncts = make([]Conjunct, 0, len(v.Conjuncts))
86
+
for _, c := range v.Conjuncts {
87
+
node := stripNonDefaultsNode(c)
88
+
w.Conjuncts = append(w.Conjuncts, MakeRootConjunct(c.Env, node))
83
89
}
84
90
return w
85
91
···
98
104
}
99
105
}
100
106
107
+
// stripNonDefaults removes non-default values from disjunctions in the given
108
+
// expression. It returns the modified expression and whether any stripping
109
+
// occurred. For example, `*1 | 2 | 3` becomes `1`.
110
+
//
101
111
// TODO: this should go: record preexpanded disjunctions in Vertex.
102
112
func stripNonDefaults(elem Elem) (r Elem, stripped bool) {
103
113
expr, ok := elem.(Expr)
···
155
165
return x, false
156
166
}
157
167
}
168
+
169
+
// stripNonDefaultsNode is like stripNonDefaults but operates on a Conjunct
170
+
// and preserves field wrappers (Field, LetField, etc.) so that field metadata
171
+
// is retained when computing defaults.
172
+
func stripNonDefaultsNode(c Conjunct) Node {
173
+
switch x := c.x.(type) {
174
+
case *Field:
175
+
if expr, stripped := stripNonDefaults(x.Value); stripped {
176
+
f := *x
177
+
f.Value = expr.(Expr)
178
+
return &f
179
+
}
180
+
return x
181
+
case *LetField:
182
+
if expr, stripped := stripNonDefaults(x.Value); stripped {
183
+
f := *x
184
+
f.Value = expr.(Expr)
185
+
return &f
186
+
}
187
+
return x
188
+
case *BulkOptionalField:
189
+
if expr, stripped := stripNonDefaults(x.Value); stripped {
190
+
f := *x
191
+
f.Value = expr.(Expr)
192
+
return &f
193
+
}
194
+
return x
195
+
case *DynamicField:
196
+
if expr, stripped := stripNonDefaults(x.Value); stripped {
197
+
f := *x
198
+
f.Value = expr.(Expr)
199
+
return &f
200
+
}
201
+
return x
202
+
case *Ellipsis:
203
+
if x.Value == nil {
204
+
return x
205
+
}
206
+
if expr, stripped := stripNonDefaults(x.Value); stripped {
207
+
e := *x
208
+
e.Value = expr.(Expr)
209
+
return &e
210
+
}
211
+
return x
212
+
default:
213
+
if elem, stripped := stripNonDefaults(c.Elem()); stripped {
214
+
return elem
215
+
}
216
+
return c.x
217
+
}
218
+
}
+74
internal/core/adt/default_test.go
+74
internal/core/adt/default_test.go
···
1
+
// Copyright 2025 CUE Authors
2
+
//
3
+
// Licensed under the Apache License, Version 2.0 (the "License");
4
+
// you may not use this file except in compliance with the License.
5
+
// You may obtain a copy of the License at
6
+
//
7
+
// http://www.apache.org/licenses/LICENSE-2.0
8
+
//
9
+
// Unless required by applicable law or agreed to in writing, software
10
+
// distributed under the License is distributed on an "AS IS" BASIS,
11
+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12
+
// See the License for the specific language governing permissions and
13
+
// limitations under the License.
14
+
15
+
package adt_test
16
+
17
+
import (
18
+
"sync"
19
+
"testing"
20
+
21
+
"cuelang.org/go/cue"
22
+
"cuelang.org/go/cue/cuecontext"
23
+
)
24
+
25
+
// TestDefaultConcurrent tests that calling Default() concurrently on a shared
26
+
// value with a disjunction is race-free. This is a regression test for
27
+
// https://github.com/cue-lang/cue/issues/2733
28
+
//
29
+
// The race occurred when multiple goroutines called Default() on a vertex with
30
+
// a single default (NumDefaults == 1), causing concurrent modification of the
31
+
// Conjuncts slice.
32
+
func TestDefaultConcurrent(t *testing.T) {
33
+
ctx := cuecontext.New()
34
+
35
+
// Create a value with a disjunction that has exactly 1 default.
36
+
// This triggers the NumDefaults == 1 case in Vertex.Default().
37
+
v := ctx.CompileString(`a: *"default" | "other"`)
38
+
if err := v.Err(); err != nil {
39
+
t.Fatal(err)
40
+
}
41
+
42
+
a := v.LookupPath(cue.ParsePath("a"))
43
+
if err := a.Err(); err != nil {
44
+
t.Fatal(err)
45
+
}
46
+
47
+
// Call Default() concurrently from multiple goroutines.
48
+
// Without the fix, this would cause a data race on the Conjuncts slice.
49
+
var wg sync.WaitGroup
50
+
for range 100 {
51
+
wg.Add(1)
52
+
go func() {
53
+
defer wg.Done()
54
+
for range 100 {
55
+
d, ok := a.Default()
56
+
if !ok {
57
+
t.Error("expected default to exist")
58
+
return
59
+
}
60
+
// Verify the default value is correct
61
+
s, err := d.String()
62
+
if err != nil {
63
+
t.Error(err)
64
+
return
65
+
}
66
+
if s != "default" {
67
+
t.Errorf("expected 'default', got %q", s)
68
+
return
69
+
}
70
+
}
71
+
}()
72
+
}
73
+
wg.Wait()
74
+
}