internal/core/adt: fix lifetime tracking of shared closeContext

Upon sharing of a node the processing of this node
terminates. This, in turn, leads to the closeContext
to be finalized. However, if at a later point another
conjunct invalidates the sharing, processing will
continue. For this reason, we should hold finalizing
a closeContext until all other conjuncts are processed
or up to the point sharing is undone.

Delaying closing closeContext can have an impact on
performance. In this case it should not matter, as there
is nothing to process about shared nodes.

Fixes #3062

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Id654881563eba54123c8afbd837d47bba7f53614
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1195897
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>

cue/testdata/eval/sharing.txtar

@@ -0,0 +1,57 @@
+
+
+-- in.cue --
+issue3062: ok1: {
+	#S: "a"
+	#o: x: #S
+	o: #o
+	o: X
+	X: x: A
+	A: "a"
+}
+-- out/eval/stats --
+Leaks:  0
+Freed:  11
+Reused: 4
+Allocs: 7
+Retain: 3
+
+Unifications: 11
+Conjuncts:    18
+Disjuncts:    12
+-- out/eval --
+(struct){
+  issue3062: (struct){
+    ok1: (struct){
+      #S: (string){ "a" }
+      #o: (#struct){
+        x: (string){ "a" }
+      }
+      o: (#struct){
+        x: (string){ "a" }
+      }
+      X: (struct){
+        x: (string){ "a" }
+      }
+      A: (string){ "a" }
+    }
+  }
+}
+-- out/compile --
+--- in.cue
+{
+  issue3062: {
+    ok1: {
+      #S: "a"
+      #o: {
+        x: 〈1;#S〉
+      }
+      o: 〈0;#o〉
+      o: 〈0;X〉
+      X: {
+        x: 〈1;A〉
+      }
+      A: "a"
+    }
+  }
+}

internal/core/adt/debug.go

@@ -192,6 +192,10 @@ const (
 	// DEFER is used to track recursive processing of a node.
 	DEFER // Always refers to self.
 
+	// SHARED is used to track shared nodes. The processing of shared nodes may
+	// change until all other conjuncts have been processed.
+	SHARED
+
 	// TEST is used for testing notifications.
 	TEST // Always refers to self.
 )
@@ -219,6 +223,8 @@ func (k depKind) String() string {
 		return "INIT"
 	case DEFER:
 		return "DEFER"
+	case SHARED:
+		return "SHARED"
 	case TEST:
 		return "TEST"
 	}
@@ -565,7 +571,7 @@ func (m *mermaidContext) cc(cc *closeContext) {
 				taskID = m.task(d)
 			}
 			name = fmt.Sprintf("%s((%d))", taskID, d.taskID)
-		case ROOT, INIT:
+		case ROOT, INIT, SHARED:
 			w = node
 			src := cc.src
 			if v.f != src.Label {

internal/core/adt/share.go

@@ -46,6 +46,18 @@ func (n *nodeContext) unshare() {
 	n.node.BaseValue = n.origBaseValue
 
 	n.scheduleVertexConjuncts(n.shared, v, n.sharedID)
+
+	n.sharedID.cc.decDependent(n.ctx, SHARED, n.node.cc)
+	n.sharedID.cc = nil
+}
+
+// finalizeSharing should be called when it is known for sure a node can be
+// shared.
+func (n *nodeContext) finalizeSharing() {
+	if n.sharedID.cc != nil {
+		n.sharedID.cc.decDependent(n.ctx, SHARED, n.node.cc)
+		n.sharedID.cc = nil
+	}
 }
 
 func (n *nodeContext) share(c Conjunct, arc *Vertex, id CloseInfo) {
@@ -58,6 +70,13 @@ func (n *nodeContext) share(c Conjunct, arc *Vertex, id CloseInfo) {
 	n.isShared = true
 	n.shared = c
 	n.sharedID = id
+
+	// At this point, the node may still be unshared at a later point. For this
+	// purpose we need to keep the retain count above zero until all conjuncts
+	// have been processed and it is clear that sharing is possible. Delaying
+	// such a count should not hurt performance, as a shared node is completed
+	// anyway.
+	id.cc.incDependent(n.ctx, SHARED, n.node.cc)
 }
 
 func (n *nodeContext) shareIfPossible(c Conjunct, arc *Vertex, id CloseInfo) bool {

internal/core/adt/unify.go

@@ -562,6 +562,11 @@ func (n *nodeContext) completeAllArcs(needs condition, mode runMode) bool {
 	}
 	n.node.Arcs = n.node.Arcs[:k]
 
+	// This should be called after all arcs have been processed, because
+	// whether sharing is possible or not may depend on how arcs with type
+	// ArcPending will resolve.
+	n.finalizeSharing()
+
 	// Strip struct literals that were not initialized and are not part
 	// of the output.
 	//