this repo has no description
internal/core/adt: fix panic on remnant notifications
When using the -e flag, a node may be evaluated
again. If that node leaked and was not garbage
collected, it may end up resending notifications.
If the receiving node was already cleared, this
caused a panic.
We now just check if a node has been finalized
before sending notifications. Alternatively, we
could prevent a node from being collected when
it still may receive notifications. This is not
necessary for this case.
By adding this code, though, we may paper over
a bug when a notification is dropped during normal
evaluation. We add a counter, to catch such cases.
Fixes #4004
Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I21d66f8ece11f08ba949a02028d4dab56e818b19
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1219264
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
+62
-2
+41
cmd/cue/cmd/testdata/script/export_out.txtar
+41
cmd/cue/cmd/testdata/script/export_out.txtar
···
1
+
# Issue #4004
2
+
#
3
+
# Ensure that notifications are not sent when a value has been completed.
4
+
5
+
# evalv2
6
+
env CUE_EXPERIMENT=evalv3=0
7
+
exec cue export -e out --out yaml in.cue
8
+
cmp stdout stdout_evalv2
9
+
10
+
# evalv3
11
+
env CUE_EXPERIMENT=evalv3=1
12
+
exec cue export -e out --out yaml in.cue
13
+
cmp stdout stdout_evalv3
14
+
15
+
-- in.cue --
16
+
_byKind: [string]: _
17
+
for obj in (#JobBranch & {_in: image: "foo"}).out {
18
+
_byKind: (obj.kind): obj
19
+
}
20
+
21
+
out: [for obj in _byKind {obj}]
22
+
23
+
#JobBranch: {
24
+
IN=_in: _
25
+
out: [(#JobLeaf & {_in: IN}).out]
26
+
}
27
+
#JobLeaf: {
28
+
_in: _
29
+
out: {
30
+
kind: "Job"
31
+
spec: image: _in.image
32
+
}
33
+
}
34
+
-- stdout_evalv2 --
35
+
- kind: Job
36
+
spec:
37
+
image: foo
38
+
-- stdout_evalv3 --
39
+
- kind: Job
40
+
spec:
41
+
image: foo
+4
cmd/cue/cmd/testdata/script/stats.txtar
+4
cmd/cue/cmd/testdata/script/stats.txtar
···
58
58
"GenerationMismatch": 0,
59
59
"MisalignedConjunct": 0,
60
60
"MisalignedConstraint": 0,
61
+
"SkippedNotification": 0,
61
62
"Freed": 6,
62
63
"Reused": 0,
63
64
"Allocs": 6,
···
83
84
GenerationMismatch: 0
84
85
MisalignedConjunct: 0
85
86
MisalignedConstraint: 0
87
+
SkippedNotification: 0
86
88
Freed: 6
87
89
Reused: 0
88
90
Allocs: 6
···
107
109
GenerationMismatch: 0
108
110
MisalignedConjunct: 0
109
111
MisalignedConstraint: 0
112
+
SkippedNotification: 0
110
113
Freed: 6
111
114
Reused: 0
112
115
Allocs: 6
···
130
133
"GenerationMismatch": 0,
131
134
"MisalignedConjunct": 0,
132
135
"MisalignedConstraint": 0,
136
+
"SkippedNotification": 0,
133
137
"Freed": 6,
134
138
"Reused": 0,
135
139
"Allocs": 6,
+11
-2
cue/stats/stats.go
+11
-2
cue/stats/stats.go
···
96
96
// aligned. This is more likely to be a bug.
97
97
MisalignedConstraint int64
98
98
99
+
// SkippedNotification indicates the number of notifications that were
100
+
// skipped because the value was already finalized. This may miss conjuncts
101
+
// when it occurs during evaluation, but it may also be triggered during
102
+
// dependency analysis, in which case it is benign.
103
+
SkippedNotification int64
104
+
99
105
// Buffer counters
100
106
//
101
107
// Each unification and disjunct operation is associated with an object
···
137
143
c.GenerationMismatch += other.GenerationMismatch
138
144
c.MisalignedConjunct += other.MisalignedConjunct
139
145
c.MisalignedConstraint += other.MisalignedConstraint
146
+
c.SkippedNotification += other.SkippedNotification
140
147
141
148
c.NumCloseIDs += other.NumCloseIDs
142
149
c.ConjunctInfos += other.ConjunctInfos
···
164
171
c.GenerationMismatch -= start.GenerationMismatch
165
172
c.MisalignedConjunct -= start.MisalignedConjunct
166
173
c.MisalignedConstraint -= start.MisalignedConstraint
174
+
c.SkippedNotification -= start.SkippedNotification
167
175
c.NumCloseIDs -= start.NumCloseIDs
168
176
c.ConjunctInfos -= start.ConjunctInfos
169
177
···
199
207
Unifications: {{.Unifications}}
200
208
Conjuncts: {{.Conjuncts}}
201
209
Disjuncts: {{.Disjuncts}}{{if .Notifications}}
202
-
Notifications: {{.Notifications}}{{end}}{{if or .GenerationMismatch .MisalignedConjunct .MisalignedConstraint}}
210
+
Notifications: {{.Notifications}}{{end}}{{if or .GenerationMismatch .MisalignedConjunct .MisalignedConstraint .SkippedNotification}}
203
211
{{if .GenerationMismatch}}
204
212
GenerationMismatch: {{.GenerationMismatch}}{{end}}{{if .MisalignedConjunct}}
205
213
MisalignedConjunct: {{.MisalignedConjunct}}{{end}}{{if .MisalignedConstraint}}
206
-
MisalignedConstraint: {{.MisalignedConstraint}}{{end}}{{end}}{{if .NumCloseIDs}}
214
+
MisalignedConstraint: {{.MisalignedConstraint}}{{end}}{{if .SkippedNotification}}
215
+
SkippedNotification: {{.SkippedNotification}}{{end}}{{end}}{{if .NumCloseIDs}}
207
216
208
217
NumCloseIDs: {{.NumCloseIDs}}{{end}}{{if or (ge .MaxReqSets 150) (ge .MaxConjunctInfos 8) (ge .MaxRedirect 2)}}
209
218
+6
internal/core/adt/fields.go
+6
internal/core/adt/fields.go
···
238
238
// TODO: we should probably only notify a conjunct once the root of the
239
239
// conjunct group is completed. This will make it easier to "stitch" the
240
240
// conjunct trees together, as its correctness will be guaranteed.
241
+
if rec.v.state == nil || rec.v.status == finalized {
242
+
// TODO: alternatively prevent nodes from being finalized when they
243
+
// still may receive notifications.
244
+
ctx.stats.SkippedNotification++
245
+
continue
246
+
}
241
247
rec.v.state.scheduleConjunct(c, rec.c)
242
248
}
243
249