[master] 5735a543c Insist that VCL_RET_FAIL goes through VRT_fail() and not VRT_handling()

[Poul-Henning Kamp]

commit 5735a543c720655c484ef11c70b6cde7f201d5ca
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Feb 12 10:01:00 2019 +0000

    Insist that VCL_RET_FAIL goes through VRT_fail() and not VRT_handling()

diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 45b4e1399..8940a6797 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -491,11 +491,11 @@ VRT_handling(VRT_CTX, unsigned hand)
 {
 
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
-	if (ctx->handling == NULL)
-		return;
+	assert(hand != VCL_RET_FAIL);
+	AN(ctx->handling);
+	AZ(*ctx->handling);
 	assert(hand > 0);
 	assert(hand < VCL_RET_MAX);
-	// XXX:NOTYET assert(*ctx->handling == 0);
 	*ctx->handling = hand;
 }
 
@@ -507,16 +507,21 @@ VRT_fail(VRT_CTX, const char *fmt, ...)
 	va_list ap;
 
 	assert(ctx->vsl != NULL || ctx->msg != NULL);
+	AN(ctx->handling);
+	if (*ctx->handling == VCL_RET_FAIL)
+		return;
+	AZ(*ctx->handling);
+	AN(fmt);
 	AZ(strchr(fmt, '\n'));
 	va_start(ap, fmt);
-	if (ctx->vsl != NULL)
+	if (ctx->vsl != NULL) {
 		VSLbv(ctx->vsl, SLT_VCL_Error, fmt, ap);
-	else {
+	} else {
 		VSB_vprintf(ctx->msg, fmt, ap);
 		VSB_putc(ctx->msg, '\n');
 	}
 	va_end(ap);
-	VRT_handling(ctx, VCL_RET_FAIL);
+	*ctx->handling = VCL_RET_FAIL;
 }
 
 /*--------------------------------------------------------------------
@@ -772,9 +777,8 @@ VRT_purge(VRT_CTX, VCL_DURATION ttl, VCL_DURATION grace, VCL_DURATION keep)
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 
 	if ((ctx->method & (VCL_MET_HIT|VCL_MET_MISS)) == 0) {
-		VSLb(ctx->vsl, SLT_VCL_Error,
+		VRT_fail(ctx,
 		    "purge can only happen in vcl_hit{} or vcl_miss{}");
-		VRT_handling(ctx, VCL_RET_FAIL);
 		return (0);
 	}
 
diff --git a/lib/libvcc/vcc_action.c b/lib/libvcc/vcc_action.c
index ded906870..02d323a14 100644
--- a/lib/libvcc/vcc_action.c
+++ b/lib/libvcc/vcc_action.c
@@ -350,7 +350,10 @@ vcc_act_return(struct vcc *tl, struct token *t, struct symbol *sym)
 		}
 	}
 	ERRCHK(tl);
-	Fb(tl, 1, "VRT_handling(ctx, VCL_RET_%s);\n", h);
+	if (hand == VCL_RET_FAIL)
+		Fb(tl, 1, "VRT_fail(ctx, \"Failed from VCL\");\n");
+	else
+		Fb(tl, 1, "VRT_handling(ctx, VCL_RET_%s);\n", h);
 	SkipToken(tl, ')');
 	SkipToken(tl, ';');
 }
diff --git a/lib/libvcc/vcc_compile.c b/lib/libvcc/vcc_compile.c
index 13994eb6b..203f34465 100644
--- a/lib/libvcc/vcc_compile.c
+++ b/lib/libvcc/vcc_compile.c
@@ -318,8 +318,15 @@ EmitInitFini(const struct vcc *tl)
 		if (VSB_len(p->event))
 			has_event = 1;
 	}
+
+	/* Handle failures from vcl_init */
+	Fc(tl, 0, "\n");
+	Fc(tl, 0, "\tif (*ctx->handling != VCL_RET_OK)\n");
+	Fc(tl, 0, "\t\treturn(1);\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");
@@ -655,7 +662,12 @@ vcc_CompileSource(struct vcc *tl, struct source *sp)
 
 	/* Tie vcl_init/fini in */
 	ifp = New_IniFin(tl);
-	VSB_printf(ifp->ini, "\tVGC_function_vcl_init(ctx);");
+	VSB_printf(ifp->ini, "\tVGC_function_vcl_init(ctx);\n");
+	/*
+	 * We do not return(1) if this fails, because the failure
+	 * could be half way into vcl_init{} so vcl_fini{} must
+	 * always be called, also on failure.
+	 */
 	VSB_printf(ifp->fin, "\t\tVGC_function_vcl_fini(ctx);");
 
 	/* Emit method functions */