From 16e3250cebe8308e829ef849e74b4afcedf093b1 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Wed, 29 Jan 2025 14:43:25 +0100 Subject: [PATCH] internal/core/adt: prevent recursion in calls Expressions that contain a call may end up in an infinite recursion if we do not ensure that there is non-cyclic data to propagate the evaluation. This fixes some tests, but we found it also makes it more robust against infinite recursion when developing alternative evaluation strategies. Issue #3649 Signed-off-by: Marcel van Lohuizen Change-Id: I7d6fecb886cf50f34589607dae606de1fea5ce93 Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1208008 Unity-Result: CUE porcuepine TryBot-Result: CUEcueckoo Reviewed-by: Matthew Sackman --- cue/testdata/cycle/builtins.txtar | 146 +++++++++++++++++++++--------- internal/core/adt/conjunct.go | 12 +++ internal/core/adt/cycle.go | 6 +- 3 files changed, 122 insertions(+), 42 deletions(-) diff --git a/cue/testdata/cycle/builtins.txtar b/cue/testdata/cycle/builtins.txtar index 56fbde00350..28621c40e66 100644 --- a/cue/testdata/cycle/builtins.txtar +++ b/cue/testdata/cycle/builtins.txtar @@ -209,14 +209,14 @@ selfCycle: t2: { -- todo/p1 -- issue3443.noCycle: fix hang -- out/evalalpha/stats -- -Leaks: 314 +Leaks: 302 Freed: 17 Reused: 17 -Allocs: 314 +Allocs: 302 Retain: 0 -Unifications: 272 -Conjuncts: 1059 +Unifications: 262 +Conjuncts: 1037 Disjuncts: 28 -- diff/-out/evalalpha/stats<==>+out/eval/stats -- diff old new @@ -228,17 +228,17 @@ diff old new -Reused: 357 -Allocs: 34 -Retain: 95 -+Leaks: 314 ++Leaks: 302 +Freed: 17 +Reused: 17 -+Allocs: 314 ++Allocs: 302 +Retain: 0 -Unifications: 363 -Conjuncts: 645 -Disjuncts: 454 -+Unifications: 272 -+Conjuncts: 1059 ++Unifications: 262 ++Conjuncts: 1037 +Disjuncts: 28 -- out/eval/stats -- Leaks: 22 @@ -256,14 +256,18 @@ noCycle.t1.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1 ./cycle.cue:6:7 ./cycle.cue:6:14 ./cycle.cue:7:13 -noCycle.t1.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1: +noCycle.t1.#x.#x: structural cycle: ./cycle.cue:6:7 - ./cycle.cue:6:14 - ./cycle.cue:7:13 noCycle.t1._s.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1: ./cycle.cue:6:7 ./cycle.cue:6:14 ./cycle.cue:7:13 +issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 matched, expected 1: + ./cycle.cue:33:6 + ./cycle.cue:30:11 + ./cycle.cue:33:13 +issue3649.cycle.t1.data.a.a: structural cycle: + ./cycle.cue:33:6 selfCycle.t1.c: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1: ./cycle.cue:47:5 ./cycle.cue:47:12 @@ -317,14 +321,13 @@ Result: // ./cycle.cue:6:7 // ./cycle.cue:6:14 // ./cycle.cue:7:13 - // noCycle.t1.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1: + // noCycle.t1.#x.#x: structural cycle: // ./cycle.cue:6:7 - // ./cycle.cue:6:14 - // ./cycle.cue:7:13 } } } - issue3649: (struct){ + issue3649: (_|_){ + // [eval] noCycle: (struct){ t1: (struct){ data: (#struct){ @@ -365,10 +368,19 @@ Result: } } } - cycle: (struct){ - t1: (struct){ - data: (#struct){ - a: (#struct){ + cycle: (_|_){ + // [eval] + t1: (_|_){ + // [eval] + data: (_|_){ + // [eval] + a: (_|_){ + // [eval] issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 matched, expected 1: + // ./cycle.cue:33:6 + // ./cycle.cue:30:11 + // ./cycle.cue:33:13 + // issue3649.cycle.t1.data.a.a: structural cycle: + // ./cycle.cue:33:6 b: (string){ "foo" } } b: (string){ string } @@ -623,7 +635,17 @@ Result: diff old new --- old +++ new -@@ -11,57 +11,36 @@ +@@ -3,65 +3,48 @@ + ./cycle.cue:6:7 + ./cycle.cue:6:14 + ./cycle.cue:7:13 +-noCycle.t1.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1: +- ./cycle.cue:6:7 +- ./cycle.cue:6:14 +- ./cycle.cue:7:13 ++noCycle.t1.#x.#x: structural cycle: ++ ./cycle.cue:6:7 + noCycle.t1._s.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1: ./cycle.cue:6:7 ./cycle.cue:6:14 ./cycle.cue:7:13 @@ -631,6 +653,12 @@ diff old new - ./cycle.cue:6:7 - ./cycle.cue:6:14 - ./cycle.cue:7:13 ++issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 matched, expected 1: ++ ./cycle.cue:33:6 ++ ./cycle.cue:30:11 ++ ./cycle.cue:33:13 ++issue3649.cycle.t1.data.a.a: structural cycle: ++ ./cycle.cue:33:6 selfCycle.t1.c: invalid value {d:{}} (does not satisfy matchN): 0 matched, expected 1: ./cycle.cue:47:5 ./cycle.cue:47:12 @@ -696,7 +724,7 @@ diff old new ./matchn.cue:55:13 Result: -@@ -78,10 +57,6 @@ +@@ -78,10 +61,6 @@ // ./cycle.cue:6:7 // ./cycle.cue:6:14 // ./cycle.cue:7:13 @@ -707,14 +735,35 @@ diff old new } } #x: (_|_){ -@@ -100,15 +75,15 @@ - noCycle: (struct){ - t1: (struct){ - data: (#struct){ +@@ -89,26 +68,25 @@ + // ./cycle.cue:6:7 + // ./cycle.cue:6:14 + // ./cycle.cue:7:13 +- // noCycle.t1.#x.#x: invalid value {} (does not satisfy matchN): 0 matched, expected 1: +- // ./cycle.cue:6:7 +- // ./cycle.cue:6:14 +- // ./cycle.cue:7:13 +- } +- } +- } +- issue3649: (struct){ +- noCycle: (struct){ +- t1: (struct){ +- data: (#struct){ - b?: (string){ string } - a: (struct){ - b: (string){ "foo" } - } ++ // noCycle.t1.#x.#x: structural cycle: ++ // ./cycle.cue:6:7 ++ } ++ } ++ } ++ issue3649: (_|_){ ++ // [eval] ++ noCycle: (struct){ ++ t1: (struct){ ++ data: (#struct){ + a: (#struct){ + b: (string){ "foo" } + } @@ -728,7 +777,7 @@ diff old new } }) } } -@@ -115,7 +90,6 @@ +@@ -115,7 +93,6 @@ } t2: (struct){ x: (struct){ @@ -736,7 +785,7 @@ diff old new y: (struct){ a: (struct){ d: (struct){ -@@ -123,14 +97,15 @@ +@@ -123,14 +100,15 @@ } } } @@ -754,15 +803,30 @@ diff old new } }) } } -@@ -140,15 +115,15 @@ - cycle: (struct){ - t1: (struct){ - data: (#struct){ +@@ -137,18 +115,27 @@ + } + } + } +- cycle: (struct){ +- t1: (struct){ +- data: (#struct){ - b: (string){ string } - a: (struct){ - b: (string){ "foo" } - } -+ a: (#struct){ ++ cycle: (_|_){ ++ // [eval] ++ t1: (_|_){ ++ // [eval] ++ data: (_|_){ ++ // [eval] ++ a: (_|_){ ++ // [eval] issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 matched, expected 1: ++ // ./cycle.cue:33:6 ++ // ./cycle.cue:30:11 ++ // ./cycle.cue:33:13 ++ // issue3649.cycle.t1.data.a.a: structural cycle: ++ // ./cycle.cue:33:6 + b: (string){ "foo" } + } + b: (string){ string } @@ -775,7 +839,7 @@ diff old new } }) } } -@@ -161,8 +136,8 @@ +@@ -161,8 +148,8 @@ a?: ((string|bytes)){ "encoding/yaml".Validate(yamlNoCycle.#c) } } data: (#struct){ @@ -785,7 +849,7 @@ diff old new } } selfCycle: (_|_){ -@@ -174,17 +149,7 @@ +@@ -174,17 +161,7 @@ // ./cycle.cue:47:5 // ./cycle.cue:47:12 // ./cycle.cue:48:5 @@ -804,7 +868,7 @@ diff old new // ./cycle.cue:47:5 d: (struct){ } -@@ -197,15 +162,7 @@ +@@ -197,15 +174,7 @@ // ./cycle.cue:53:5 // ./cycle.cue:53:12 // ./cycle.cue:54:5 @@ -821,7 +885,7 @@ diff old new // ./cycle.cue:53:5 } } -@@ -267,20 +224,20 @@ +@@ -267,20 +236,20 @@ issue3410: (struct){ _s: (struct){ #x: (_){ matchN(1, (#list){ @@ -850,7 +914,7 @@ diff old new } }) } } -@@ -287,9 +244,9 @@ +@@ -287,9 +256,9 @@ issue3420: (struct){ matches1: (struct){ #S: (_){ matchN(1, (#list){ @@ -863,7 +927,7 @@ diff old new } }) } s: (int){ 2 } -@@ -306,13 +263,13 @@ +@@ -306,13 +275,13 @@ } noCycle: (struct){ #S: (_){ matchN(1, (#list){ @@ -880,7 +944,7 @@ diff old new n: (struct){ n: (_){ _ } } -@@ -321,29 +278,43 @@ +@@ -321,29 +290,43 @@ } noCycle2: (struct){ #S: (_){ matchN(1, (#list){ @@ -937,7 +1001,7 @@ diff old new n: (struct){ n: (_){ _ } } -@@ -358,18 +329,7 @@ +@@ -358,18 +341,7 @@ // [eval] issue3443.cycle2.fail.#S: invalid value {n:{n:{n:_}}} (does not satisfy matchN): 0 matched, expected 1: // ./matchn.cue:55:13 // ./matchn.cue:55:20 @@ -957,7 +1021,7 @@ diff old new // ./matchn.cue:55:13 n: (#struct){ n: (#struct){ -@@ -396,11 +356,11 @@ +@@ -396,11 +368,11 @@ // ./matchn.cue:63:29 } #s: (_){ matchN(1, (#list){ diff --git a/internal/core/adt/conjunct.go b/internal/core/adt/conjunct.go index 6b2ccb836e2..8e1766f54ab 100644 --- a/internal/core/adt/conjunct.go +++ b/internal/core/adt/conjunct.go @@ -228,6 +228,18 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) { case Evaluator: n.unshare() + + // Expressions that contain a call may end up in an infinite recursion + // here if we do not ensure that there is non-cyclic data to propagate + // the evaluation. We therefore postpone expressions until we have + // evidence that such non-cyclic conjuncts exist. + if id.CycleType == IsCyclic && !n.hasNonCycle && !n.hasNonCyclic { + n.hasAncestorCycle = true + n.cyclicConjuncts = append(n.cyclicConjuncts, cyclicConjunct{c: c}) + n.node.cc().incDependent(n.ctx, DEFER, nil) + return + } + // Interpolation, UnaryExpr, CallExpr n.scheduleTask(handleExpr, env, x, id) diff --git a/internal/core/adt/cycle.go b/internal/core/adt/cycle.go index ceead753563..7be0056c362 100644 --- a/internal/core/adt/cycle.go +++ b/internal/core/adt/cycle.go @@ -1103,7 +1103,11 @@ func (n *nodeContext) updateCyclicStatusV3(c CloseInfo) { for _, c := range n.cyclicConjuncts { ci := c.c.CloseInfo ci.cc = n.node.rootCloseContext(n.ctx) - n.scheduleVertexConjuncts(c.c, c.arc, ci) + if c.arc != nil { + n.scheduleVertexConjuncts(c.c, c.arc, ci) + } else { + n.scheduleConjunct(c.c, ci) + } n.node.cc().decDependent(n.ctx, DEFER, nil) } n.cyclicConjuncts = n.cyclicConjuncts[:0]