Clarify (struct vrt_ctx).handling for PRIVs

Ref 43d9e5fb :

PRIV_* fini methods need to leave (struct vrt_ctx).handling alone,
except that they might call VRT_fail(), see also
746384b2

Thus we add assertions that handling be either 0 or VCL_RET_FAIL
outside the FSM.

To be able to do so, we need to change VCL_RET_OK into 0 when
vcl_init{} has returned successfully.

The vcl_fini{} case is slightly more complicated:

By design, only "ok" (VCL_RET_OK) is allowed, but VRT_fail() also
added VCL_RET_FAIL, so we de-facto get a "fail" return if any vmod
code called VRT_fail().

Because PRIV_* handling happens from VCC generated code via
VGC_Discard(), we need to check and change (struct vrt_ctx).handling
right after calling vcl_fini{} / VGC_function_vcl_fini() from
VGC_Discard(). This is VPI_vcl_fini().

Implementation note:

I also considered void VPI_vcl_fini(VRT_CTX, vcl_func_f fini_sub),
having VPI_vcl_fini call the fini_sub, but that stirred up includes of
VPI where vcl.h is not included.

bin/varnishd/cache/cache_vpi.c

@@ -62,6 +62,26 @@ VPI_count(VRT_CTX, unsigned u)
 		    ctx->vcl->conf->ref[u].line, ctx->vcl->conf->ref[u].pos);
 }
 
+/*
+ * After vcl_fini {} == VGC_function_vcl_fini() is called from VGC_Discard(),
+ * handling must either be OK from VCL "return (ok)" or FAIL from VRT_fail().
+ *
+ * replace OK with 0 for _fini callbacks because that handling has meaning only
+ * when returning from VCL subs
+ */
+
+void
+VPI_vcl_fini(VRT_CTX)
+{
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+	AN(ctx->handling);
+
+	if (*ctx->handling == VCL_RET_FAIL)
+		return;
+	assert(*ctx->handling == VCL_RET_OK);
+	*ctx->handling = 0;
+}
+
 VCL_VCL
 VPI_vcl_get(VRT_CTX, const char *name)
 {

bin/varnishd/cache/cache_vrt_priv.c

@@ -277,6 +277,7 @@ VRT_priv_fini(VRT_CTX, const struct vmod_priv *p)
 	VRT_CTX_Assert(ctx);
 
 	m->fini(ctx, p->priv);
+	assert(*ctx->handling == 0 || *ctx->handling == VCL_RET_FAIL);
 }
 
 /*--------------------------------------------------------------------*/
@@ -293,6 +294,10 @@ VCL_TaskLeave(VRT_CTX, struct vrt_privs *privs)
 {
 	struct vrt_priv *vp, *vp1;
 
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+	AN(ctx->handling);
+	assert(*ctx->handling == 0 || *ctx->handling == VCL_RET_FAIL);
+
 	/*
 	 * NB: We don't bother removing entries as we finish them because it's
 	 * a costly operation. Instead we safely walk the whole tree and clear

include/vcc_interface.h

@@ -52,6 +52,7 @@ struct vpi_ref {
 };
 
 void VPI_count(VRT_CTX, unsigned);
+void VPI_vcl_fini(VRT_CTX);
 
 int VPI_Vmod_Init(VRT_CTX, struct vmod **hdl, unsigned nbr, void *ptr, int len,
     const char *nm, const char *path, const char *file_id, const char *backup);

lib/libvcc/vcc_compile.c

@@ -364,10 +364,10 @@ EmitInitFini(const struct vcc *tl)
 	Fc(tl, 0, "\n");
 	Fc(tl, 0, "\tif (*ctx->handling != VCL_RET_OK)\n");
 	Fc(tl, 0, "\t\treturn(1);\n");
+	Fc(tl, 0, "\t*ctx->handling = 0;\n");
 
 	VTAILQ_FOREACH(sy, &tl->sym_objects, sideways) {
 		Fc(tl, 0, "\tif (!%s) {\n", sy->rname);
-		Fc(tl, 0, "\t\t*ctx->handling = 0;\n");
 		Fc(tl, 0, "\t\tVRT_fail(ctx, "
 		    "\"Object %s not initialized\");\n" , sy->name);
 		Fc(tl, 0, "\t\treturn(1);\n");
@@ -741,7 +741,7 @@ vcc_CompileSource(struct vcc *tl, struct source *sp, const char *jfile)
 	 * must always be called, also on failure.
 	 */
 	ifp->ignore_errors = 1;
-	VSB_cat(ifp->fin, "\t\tVGC_function_vcl_fini(ctx);");
+	VSB_cat(ifp->fin, "\t\tVGC_function_vcl_fini(ctx);VPI_vcl_fini(ctx);");
 
 	/* Emit method functions */
 	Fh(tl, 1, "\n");