internal/core/adt: fix benign memory leak

This fixes a memory leak that would cause a nodeContext
to be not recycled. As the nodeContext would have been
garbage collected anyway, this was just a matter of
performance.

A leak checker has been added to the tests. There are
some seemingly benign leaks remaining, so the test
uses Skipf for now.

Change-Id: I172a5a9f76e30a69cca398bd14f243ee13d96765
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8304
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>

internal/core/adt/disjunct.go

@@ -330,6 +330,7 @@ func mode(hasDefault, marked bool) defaultMode {
 // time that a clone is needed and must be nil. Conjuncts no longer needed and
 // can become nil. All other fields can be copied shallowly.
 func clone(v Vertex) Vertex {
+	v.state = nil
 	if a := v.Arcs; len(a) > 0 {
 		v.Arcs = make([]*Vertex, len(a))
 		for i, arc := range a {

internal/core/adt/eval.go

@@ -57,7 +57,18 @@ type Stats struct {
 	Allocs   int
 }
 
+// Leaks reports the number of nodeContext structs leaked. These are typically
+// benign, as they will just be garbage collected, as long as the pointer from
+// the original nodes has been eliminated or the original nodes are also not
+// referred to. But Leaks may have notable impact on performance, and thus
+// should be avoided.
+func (s *Stats) Leaks() int {
+	return s.Allocs + s.Reused - s.Freed
+}
+
 var stats = template.Must(template.New("stats").Parse(`{{"" -}}
+
+Leaks:  {{.Leaks}}
 Freed:  {{.Freed}}
 Reused: {{.Reused}}
 Allocs: {{.Allocs}}
@@ -330,10 +341,6 @@ func (e *Unifier) Unify(c *OpContext, v *Vertex, state VertexStatus) {
 		}
 		n.expandDisjuncts(disState, n, maybeDefault, false)
 
-		// If the state has changed, it is because a disjunct has been run. In this case, our node will have completed, and it will
-		// set a value soon.
-		v.state = n // alternatively, set to nil
-
 		for _, d := range n.disjuncts {
 			d.free()
 		}
@@ -373,6 +380,11 @@ func (e *Unifier) Unify(c *OpContext, v *Vertex, state VertexStatus) {
 			// TODO: how to represent closedness information? Do we need it?
 		}
 
+		// If the state has changed, it is because a disjunct has been run, or
+		// because a single disjunct has replaced it. Restore the old state as
+		// to not confuse memory management.
+		v.state = n
+
 		if state != Finalized {
 			return
 		}

internal/core/adt/eval_test.go

@@ -64,7 +64,11 @@ func TestEval(t *testing.T) {
 		ctx := e.NewContext(v)
 		v.Finalize(ctx)
 
-		t.Log(e.Stats())
+		stats := ctx.Stats()
+		t.Log(stats)
+		if n := stats.Leaks(); n > 0 {
+			t.Skipf("%d leaks reported", n)
+		}
 
 		if b := validate.Validate(ctx, v, &validate.Config{
 			AllErrors: true,