Mailing List Archive

commit 4ccd356d1bdcd49af4c76399c492c3d278940324
Author: Nils Goroll <nils.goroll@uplex.de>
Date: Wed Jan 20 14:16:11 2021 +0100

Clarify (struct vrt_ctx).handling for PRIVs

Ref 43d9e5fb1a10a88ab6a5a98ad4038438025c4999 :

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

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.

diff --git a/bin/varnishd/cache/cache_vpi.c b/bin/varnishd/cache/cache_vpi.c
index 3b6783ba8..06eb802d5 100644
--- a/bin/varnishd/cache/cache_vpi.c
+++ b/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)
{
diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c
index b265e061b..40434dc6e 100644
--- a/bin/varnishd/cache/cache_vrt_priv.c
+++ b/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
diff --git a/include/vcc_interface.h b/include/vcc_interface.h
index 1e5f9c3cd..865c8504e 100644
--- a/include/vcc_interface.h
+++ b/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);
diff --git a/lib/libvcc/vcc_compile.c b/lib/libvcc/vcc_compile.c
index 6deaff9e4..a7b2a690a 100644
--- a/lib/libvcc/vcc_compile.c
+++ b/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");
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit