internal/core/adt: avoid crash

Sometimes, evaluation of a Vertex may, recursively,
trigger the evaluation of itself. This may cause
the checks or Arc slices to be reset while they are
in the process of being filtered. As filtering is no
longer needed if a node is already finalized, we can
safely drop out of the computation if we detect this.

Note that there are no test cases. Issues around this
were detected by upcoming changes that fix hangs
in cyclic function and validator calls. We separate out
these changes, though, so that we can see they are
a noop and to facilitate bisection if these turn out to
be problamatic after all.

Issue #3649

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: If298ba3d9bd4addde0ac1d55be968211660bc113
Dispatch-Trailer: {"type":"trybot","CL":1208699,"patchset":2,"ref":"refs/changes/99/1208699/2","targetBranch":"master"}

internal/core/adt/conjunct.go

@@ -784,7 +784,12 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf
 
 		for i, y := range n.checks {
 			if b, ok := SimplifyValidator(ctx, cx, y); ok {
-				n.checks[i] = b
+				// It is possible that simplification process triggered further
+				// evaluation, finalizing this node and clearing the checks
+				// slice. In that case it is safe to ignore the result.
+				if len(n.checks) > 0 {
+					n.checks[i] = b
+				}
 				return
 			}
 		}

internal/core/adt/eval.go

@@ -860,6 +860,12 @@ func (n *nodeContext) completeArcs(state vertexStatus) {
 				}
 			}
 
+			// If a structural cycle is detected, Arcs is cleared to avoid
+			// going into an infinite loop. If this is the case, we can bail
+			// from this loop.
+			if len(n.node.Arcs) == 0 {
+				goto postChecks
+			}
 			n.node.Arcs[k] = a
 			k++
 
@@ -889,6 +895,7 @@ func (n *nodeContext) completeArcs(state vertexStatus) {
 		}
 		n.node.Arcs = n.node.Arcs[:k]
 
+	postChecks:
 		for _, c := range n.postChecks {
 			f := ctx.PushState(c.env, c.expr.Source())