Skip to content

Commit 19e2c74

Browse files
committed
internal/core/adt: add generation tracking to prevent stale context usage
This change adds a generation counter to OpContext and nodeContext to detect when stale nodeContexts are being used across different evaluation contexts. Each OpContext gets a unique generation ID, and nodeContexts inherit this generation when created. This helps identify bugs where nodeContexts from previous evaluations are incorrectly reused, which can lead to evaluation errors and inconsistent results. Down the line we will use this to invalidate closedness info in conjuncts. Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I860b10df46c3b35c6317dcd686ebac8cc061ed46 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1218834 TryBot-Result: CUEcueckoo <[email protected]> Unity-Result: CUE porcuepine <[email protected]> Reviewed-by: Daniel Martí <[email protected]>
1 parent 5f4bd37 commit 19e2c74

File tree

6 files changed

+54
-21
lines changed

6 files changed

+54
-21
lines changed

cmd/cue/cmd/testdata/script/stats.txtar

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ contents overwritten
5454
"ConjunctInfos": 6,
5555
"MaxConjunctInfos": 2,
5656
"MaxReqSets": 0,
57+
"GenerationMismatch": 0,
5758
"Freed": 6,
5859
"Reused": 0,
5960
"Allocs": 6,
@@ -66,19 +67,20 @@ contents overwritten
6667
}
6768
-- out/stats.cue --
6869
CUE: {
69-
EvalVersion: 3
70-
Unifications: 4
71-
Disjuncts: 2
72-
Notifications: 0
73-
Conjuncts: 8
74-
NumCloseIDs: 2
75-
ConjunctInfos: 6
76-
MaxConjunctInfos: 2
77-
MaxReqSets: 0
78-
Freed: 6
79-
Reused: 0
80-
Allocs: 6
81-
Retained: 0
70+
EvalVersion: 3
71+
Unifications: 4
72+
Disjuncts: 2
73+
Notifications: 0
74+
Conjuncts: 8
75+
NumCloseIDs: 2
76+
ConjunctInfos: 6
77+
MaxConjunctInfos: 2
78+
MaxReqSets: 0
79+
GenerationMismatch: 0
80+
Freed: 6
81+
Reused: 0
82+
Allocs: 6
83+
Retained: 0
8284
}
8385
Go: {
8486
AllocBytes: 300456
@@ -95,6 +97,7 @@ CUE:
9597
ConjunctInfos: 6
9698
MaxConjunctInfos: 2
9799
MaxReqSets: 0
100+
GenerationMismatch: 0
98101
Freed: 6
99102
Reused: 0
100103
Allocs: 6
@@ -114,6 +117,7 @@ Go:
114117
"ConjunctInfos": 6,
115118
"MaxConjunctInfos": 2,
116119
"MaxReqSets": 0,
120+
"GenerationMismatch": 0,
117121
"Freed": 6,
118122
"Reused": 0,
119123
"Allocs": 6,

cue/stats/stats.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ type Counts struct {
7575
MaxConjunctInfos int64 // Maximum number of conjunct infos in a node
7676
MaxReqSets int64 // Number of replace IDs processed
7777

78+
// Exception counters
79+
//
80+
// These counters track exceptional conditions that occur during evaluation.
81+
82+
// GenerationMismatch indicates the number of times a node was unified
83+
// with a different generation than the one it was created in.
84+
GenerationMismatch int64 // Number of exceptional unification cases
85+
7886
// Buffer counters
7987
//
8088
// Each unification and disjunct operation is associated with an object
@@ -113,6 +121,8 @@ func (c *Counts) Add(other Counts) {
113121
c.Disjuncts += other.Disjuncts
114122
c.Notifications += other.Notifications
115123

124+
c.GenerationMismatch += other.GenerationMismatch
125+
116126
c.NumCloseIDs += other.NumCloseIDs
117127
c.ConjunctInfos += other.ConjunctInfos
118128
if other.MaxConjunctInfos > c.MaxConjunctInfos {
@@ -133,6 +143,7 @@ func (c Counts) Since(start Counts) Counts {
133143
c.Conjuncts -= start.Conjuncts
134144
c.Disjuncts -= start.Disjuncts
135145
c.Notifications -= start.Notifications
146+
c.GenerationMismatch -= start.GenerationMismatch
136147
c.NumCloseIDs -= start.NumCloseIDs
137148

138149
c.ConjunctInfos -= start.ConjunctInfos
@@ -166,7 +177,9 @@ Retain: {{.Retained}}
166177
Unifications: {{.Unifications}}
167178
Conjuncts: {{.Conjuncts}}
168179
Disjuncts: {{.Disjuncts}}{{if .Notifications}}
169-
Notifications: {{.Notifications}}{{end}}{{if .NumCloseIDs}}
180+
Notifications: {{.Notifications}}{{end}}{{if .GenerationMismatch}}
181+
182+
GenerationMismatch: {{.GenerationMismatch}}{{end}}{{if .NumCloseIDs}}
170183
171184
NumCloseIDs: {{.NumCloseIDs}}{{end}}{{if or (ge .MaxReqSets 150) (ge .MaxConjunctInfos 8)}}
172185

internal/core/adt/closed.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ type closeStats struct {
309309
// the other fields of this closeStats value are only valid if generation
310310
// is equal to the generation in OpContext. This allows for lazy
311311
// initialization of closeStats.
312-
generation int
312+
generation uint64
313313

314314
// These counts keep track of how many required child nodes need to be
315315
// completed before this node is accepted.
@@ -352,7 +352,7 @@ func Accept(ctx *OpContext, n *Vertex, f Feature) (found, required bool) {
352352
if ctx.isDevVersion() {
353353
return n.accept(ctx, f), true
354354
}
355-
ctx.generation++
355+
ctx.opID++
356356
ctx.todo = nil
357357

358358
var optionalTypes OptionalType
@@ -488,8 +488,8 @@ func getScratch(ctx *OpContext, s *closeInfo) *closeStats {
488488
m[s] = x
489489
}
490490

491-
if x.generation != ctx.generation {
492-
*x = closeStats{generation: ctx.generation}
491+
if x.generation != ctx.opID {
492+
*x = closeStats{generation: ctx.opID}
493493
}
494494

495495
return x

internal/core/adt/context.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"fmt"
1919
"reflect"
2020
"regexp"
21+
"sync/atomic"
2122

2223
"github.com/cockroachdb/apd/v3"
2324
"golang.org/x/text/encoding/unicode"
@@ -59,13 +60,16 @@ type Config struct {
5960
Format func(Runtime, Node) string
6061
}
6162

63+
var contextGeneration atomic.Uint64
64+
6265
// New creates an operation context.
6366
func New(v *Vertex, cfg *Config) *OpContext {
6467
if cfg.Runtime == nil {
6568
panic("nil Runtime")
6669
}
6770

6871
ctx := &OpContext{
72+
opID: contextGeneration.Add(1),
6973
Runtime: cfg.Runtime,
7074
Format: cfg.Format,
7175
vertex: v,
@@ -76,6 +80,9 @@ func New(v *Vertex, cfg *Config) *OpContext {
7680
if v != nil {
7781
ctx.e = &Environment{Up: nil, Vertex: v}
7882
}
83+
if ctx.LogEval > 0 {
84+
ctx.Logf(v, "New context at opID %d", ctx.opID)
85+
}
7986
return ctx
8087
}
8188

@@ -142,9 +149,9 @@ type OpContext struct {
142149
// TODO(perf): have two generations: one for each pass of the closedness
143150
// algorithm, so that the results of the first pass can be reused for all
144151
// features of a node.
145-
generation int
146-
closed map[*closeInfo]*closeStats
147-
todo *closeStats
152+
opID uint64
153+
closed map[*closeInfo]*closeStats
154+
todo *closeStats
148155

149156
// evalDepth indicates the current depth of evaluation. It is used to
150157
// detect structural cycles and their severity.s

internal/core/adt/eval.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,10 @@ type arcKey struct {
10131013
type nodeContext struct {
10141014
nextFree *nodeContext
10151015

1016+
// opID is assigned the opID of the OpContext upon creation.
1017+
// This allows checking that we are not using stale nodeContexts.
1018+
opID uint64
1019+
10161020
// refCount:
10171021
// evalv2: keeps track of all current usages of the node, such that the
10181022
// node can be freed when the counter reaches zero.
@@ -1406,6 +1410,7 @@ func (c *OpContext) newNodeContext(node *Vertex) *nodeContext {
14061410
}
14071411
}
14081412

1413+
n.opID = c.opID
14091414
n.scheduler.node = n
14101415
n.underlying = node
14111416
if p := node.Parent; p != nil && p.state != nil {

internal/core/adt/unify.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode, checkTypos b
145145
return false
146146
}
147147

148+
if n := v.state; n != nil && n.ctx.opID != c.opID {
149+
c.stats.GenerationMismatch++
150+
}
151+
148152
// Note that the state of a node can be removed before the node is.
149153
// This happens with the close builtin, for instance.
150154
// See TestFromAPI in pkg export.

0 commit comments

Comments
 (0)