[master] 7d06b8b98 Call VRT_priv_{task, top} for each PRIV_{TASK, TOP} argument

Nils Goroll nils.goroll at uplex.de
Fri Mar 1 16:11:06 UTC 2024


commit 7d06b8b98350de557568d712ff089d3c961d0618
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Wed Feb 28 11:31:16 2024 +0100

    Call VRT_priv_{task,top} for each PRIV_{TASK,TOP} argument
    
    to avoid using stale pointers after a rollback.
    
    Before this change, we would call VRT_priv_* only once per subroutine,
    which can be *) a nice performance optimization, but leaves us with stale
    pointers after a rollback.
    
    Rather than adding complications for the rollback case just to keep the
    option of the "per subroutine pointer cache", just retrieve a fresh
    priv pointer every time.
    
    The other use of the per subroutine initialization was error handling,
    which needs additional code outside the function arguments, simply
    because a return statement is not possible within function arguments. We
    removed the requirement for error handling in the previous commit by
    making sure that VRT_priv_{task,top} always return a valid pointer.
    
    Fixes https://github.com/varnish/varnish-modules/issues/222
    
    Alternative implementation to #4060
    
    *) It is not an optimization in all cases, for example the priv pointers
       were intialized unconditionally, even if code using them was not
       reached - but then again, this is something C compilers might
       optimize...

diff --git a/lib/libvcc/vcc_expr.c b/lib/libvcc/vcc_expr.c
index 1e94dbf76..8cda6f12d 100644
--- a/lib/libvcc/vcc_expr.c
+++ b/lib/libvcc/vcc_expr.c
@@ -401,7 +401,6 @@ vcc_priv_arg(struct vcc *tl, const char *p, struct symbol *sym)
 	char buf[64];
 	struct inifin *ifp;
 	const char *f = NULL;
-	struct procprivhead *marklist = NULL;
 
 	AN(sym);
 	AN(sym->vmod_name);
@@ -417,31 +416,18 @@ vcc_priv_arg(struct vcc *tl, const char *p, struct symbol *sym)
 		return (vcc_mk_expr(VOID, "&%s", buf));
 	}
 
-	if (!strcmp(p, "PRIV_TASK")) {
+	if (!strcmp(p, "PRIV_TASK"))
 		f = "task";
-		marklist = &tl->curproc->priv_tasks;
-	} else if (!strcmp(p, "PRIV_TOP")) {
+	else if (!strcmp(p, "PRIV_TOP")) {
 		f = "top";
 		sym->r_methods &= VCL_MET_TASK_C;
-		marklist = &tl->curproc->priv_tops;
 	} else {
 		WRONG("Wrong PRIV_ type");
 	}
 	AN(f);
-	AN(marklist);
-	bprintf(buf, "ARG_priv_%s_%s", f, sym->vmod_name);
-
-	if (vcc_MarkPriv(tl, marklist, sym->vmod_name) == NULL)
-		VSB_printf(tl->curproc->prologue,
-			   "  struct vmod_priv *%s = "
-			   "VRT_priv_%s(ctx, &VGC_vmod_%s);\n"
-			   "  if (%s == NULL) {\n"
-			   "    VRT_fail(ctx, \"failed to get %s priv "
-			   "for vmod %s\");\n"
-			   "    return;\n"
-			   "  }\n",
-			   buf, f, sym->vmod_name, buf, f, sym->vmod_name);
-	return (vcc_mk_expr(VOID, "%s", buf));
+
+	return (vcc_mk_expr(VOID, "VRT_priv_%s(ctx, &VGC_vmod_%s)",
+	    f, sym->vmod_name));
 }
 
 struct func_arg {


More information about the varnish-commit mailing list