move a racy assertion to a safe place

assert_node is only safe under the bytes_tree lock, yet, for the error
case, we called it while other threads could still be running.

Move it until until after we know that all other tasks are done.

This also implies a second asser_node for the retval == 0 case, which
shouldn't do any harm.

should fix:

Panic at: Thu, 24 Oct 2019 14:21:27 GMT
Assert error in assert_node(), node_assert.h line 103:
  Condition((node->nexus.owner) != 0) not true.
version = varnish-6.2.1 revision
9f8588e4ab785244e06c3446fe09bf9db5dd8753, vrt api = 9.0
ident =
Linux,3.10.0-1062.4.1.el7.x86_64,x86_64,-junix,-smalloc,-smalloc,-hcritbit,epoll
now = 29366.422728 (mono), 1571926828.402512 (real)
Backtrace:
  0x43cf3b: /usr/sbin/varnishd() [0x43cf3b]
  0x4a01c2: /usr/sbin/varnishd(VAS_Fail+0x42) [0x4a01c2]
  0x7f6bf06a033c:
./vmod_cache/_vmod_pesi.4d9e0603bac2a2e2b2627f7fe90ff1d55d4759545517c869a5571f16636e230e(+0xe33c)
[0x7f6bf06a033c]
  0x4222b6: /usr/sbin/varnishd(VDP_close+0x66) [0x4222b6]
...

(gdb) bt
 #0  0x00007f6db97d2337 in __GI_raise (sig=sig@entry=6) at
 ../nptl/sysdeps/unix/sysv/linux/raise.c:55
 #1  0x00007f6db97d3a28 in __GI_abort () at abort.c:90
 #2  0x000000000043d232 in pan_ic ()
 #3  0x00000000004a01c2 in VAS_Fail ()
 #4  0x00007f6bf06a033c in assert_node (check=CHK_ANY, node=<optimized out>) at node_assert.h:103
 #5  vdp_pesi_fini (req=0x7f6ba8f52020, priv=0x7f6ba8f57aa8) at vdp_pesi.c:782
 #6  0x00000000004222b6 in VDP_close ()
 #7  0x0000000000464c5e in V1D_Deliver ()
 #8  0x0000000000441eab in CNT_Request ()
 #9  0x00000000004665b3 in http1_req ()
 #10 0x000000000045c833 in WRK_Thread ()
 #11 0x000000000045ccf0 in pool_thread ()
 #12 0x00007f6db9b71e65 in start_thread (arg=0x7f6cdc5dc700) at
 pthread_create.c:307
 #13 0x00007f6db989a88d in clone () at
 ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
 (gdb) f 4
 #4  0x00007f6bf06a033c in assert_node (check=CHK_ANY, node=<optimized out>) at node_assert.h:103
 103                     AN(node->nexus.owner);

src/vdp_pesi.c

@@ -774,24 +774,20 @@ vdp_pesi_fini(struct req *req, void **priv)
 	assert(req->transport_priv == NULL ||
 	       *(unsigned *)req->transport_priv != PESI_MAGIC);
 
-	/*
-	 * at this point, the tree must be delivered completely, but tasks using
-	 * our shared data structures could still be running
-	 */
-	if (bytes_tree->retval) {
-		assert_node(bytes_tree->root, CHK_ANY);
-	}
-	else if (bytes_tree->root->nexus.npending_private == 1) {
-		/* our VDP has not run - e.g. HEAD request */
-		assert(bytes_tree->npending == 0);
+	if (bytes_tree->root->nexus.npending_private == 1 &&
+	    bytes_tree->npending == 0) {
+		AZ(bytes_tree->retval);
 		assert(bytes_tree->root->state == ST_PRIVATE);
 		assert(bytes_tree->root->type == T_NEXUS);
-	}
-	else {
+	} else if (bytes_tree->retval == 0) {
 		assert(bytes_tree->root->nexus.npending_private == 0);
 		assert(bytes_tree->npending == 0);
 		assert_node(bytes_tree->root, CHK_DELI);
 	}
+	/*
+	 * else we got an error condition and cannot make assumptions about the
+	 * state of the tree before all tasks have finished
+	 */
 
 	CHECK_OBJ_NOTNULL(pesi_tree, PESI_TREE_MAGIC);
 	assert(pesi_tree == pesi->pesi_tree);
@@ -815,6 +811,9 @@ vdp_pesi_fini(struct req *req, void **priv)
 		Lck_Unlock(&pesi_tree->task_lock);
 	}
 
+	/* no locking required because no other tasks are running */
+	assert_node(bytes_tree->root, CHK_ANY);
+
 	/*
 	 * for an early client close, our subrequests had no chance to run
 	 *