[master] d6d341609 Revert "actually, the scope must not change either"
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Fri May 24 18:22:11 UTC 2019
commit d6d3416090b8b7db62d2cd0958e756e23ea92352
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Fri May 24 19:29:55 2019 +0200
Revert "actually, the scope must not change either"
This reverts commit 86af5ce095a6e39e313cf7c6c23a49edba4aace6.
Unfortunately we have two scopes top and task that may conflict when
esi_level is zero. This restores the id field, making struct vrt_priv
slightly larger than when it used to be managed as a tailq. But in
the context of a red-black tree for dynamic priv lookups the benefits
should still outweigh the slight increase in memory footprint.
Conflicts:
bin/varnishd/cache/cache_vrt_priv.c
Fixes #3003
diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c
index daa667aab..76152cbf3 100644
--- a/bin/varnishd/cache/cache_vrt_priv.c
+++ b/bin/varnishd/cache/cache_vrt_priv.c
@@ -42,6 +42,7 @@ struct vrt_priv {
#define VRT_PRIV_MAGIC 0x24157a52
VRBT_ENTRY(vrt_priv) entry;
struct vmod_priv priv[1];
+ uintptr_t id;
uintptr_t vmod_id;
};
@@ -95,9 +96,9 @@ VRTPRIV_init(struct vrt_privs *privs)
static inline int
vrt_priv_dyncmp(const struct vrt_priv *vp1, const struct vrt_priv *vp2)
{
- if (vp1->vmod_id < vp2->vmod_id)
+ if (vp1->vmod_id < vp2->vmod_id || vp1->id < vp2->id)
return (-1);
- if (vp1->vmod_id > vp2->vmod_id)
+ if (vp1->vmod_id > vp2->vmod_id || vp1->id > vp2->id)
return (1);
return (0);
}
@@ -105,17 +106,23 @@ vrt_priv_dyncmp(const struct vrt_priv *vp1, const struct vrt_priv *vp2)
VRBT_GENERATE_STATIC(vrt_priv_tree, vrt_priv, entry, vrt_priv_dyncmp)
static struct vmod_priv *
-vrt_priv_dynamic(struct ws *ws, struct vrt_privs *vps, uintptr_t vmod_id)
+vrt_priv_dynamic(struct ws *ws, struct vrt_privs *vps, uintptr_t id,
+ uintptr_t vmod_id)
{
struct vrt_priv *vp;
- const struct vrt_priv needle = {.vmod_id = vmod_id};
+ const struct vrt_priv needle = {
+ .id = id,
+ .vmod_id = vmod_id,
+ };
CHECK_OBJ_NOTNULL(vps, VRT_PRIVS_MAGIC);
+ AN(id);
AN(vmod_id);
vp = VRBT_FIND(vrt_priv_tree, &vps->privs, &needle);
if (vp) {
CHECK_OBJ(vp, VRT_PRIV_MAGIC);
+ assert(vp->id == id);
assert(vp->vmod_id == vmod_id);
return (vp->priv);
}
@@ -124,6 +131,7 @@ vrt_priv_dynamic(struct ws *ws, struct vrt_privs *vps, uintptr_t vmod_id)
if (vp == NULL)
return (NULL);
INIT_OBJ(vp, VRT_PRIV_MAGIC);
+ vp->id = id;
vp->vmod_id = vmod_id;
VRBT_INSERT(vrt_priv_tree, &vps->privs, vp);
return (vp->priv);
@@ -132,6 +140,7 @@ vrt_priv_dynamic(struct ws *ws, struct vrt_privs *vps, uintptr_t vmod_id)
struct vmod_priv *
VRT_priv_task(VRT_CTX, const void *vmod_id)
{
+ uintptr_t id;
struct vrt_privs *vps;
struct vmod_priv *vp;
@@ -141,34 +150,40 @@ VRT_priv_task(VRT_CTX, const void *vmod_id)
if (ctx->req) {
CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
+ id = (uintptr_t)ctx->req;
CAST_OBJ_NOTNULL(vps, ctx->req->privs, VRT_PRIVS_MAGIC);
} else if (ctx->bo) {
CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
+ id = (uintptr_t)ctx->bo;
CAST_OBJ_NOTNULL(vps, ctx->bo->privs, VRT_PRIVS_MAGIC);
} else {
ASSERT_CLI();
+ id = (uintptr_t)cli_task_privs;
CAST_OBJ_NOTNULL(vps, cli_task_privs, VRT_PRIVS_MAGIC);
}
- vp = vrt_priv_dynamic(ctx->ws, vps, (uintptr_t)vmod_id);
+ vp = vrt_priv_dynamic(ctx->ws, vps, id, (uintptr_t)vmod_id);
return (vp);
}
struct vmod_priv *
VRT_priv_top(VRT_CTX, const void *vmod_id)
{
+ uintptr_t id;
struct vrt_privs *vps;
struct req *req;
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
if (ctx->req == NULL) {
+ /* XXX: should we VRT_fail here instead? */
WRONG("PRIV_TOP is only accessible in client VCL context");
NEEDLESS(return (NULL));
}
CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
req = ctx->req->topreq;
+ id = (uintptr_t)&req->topreq;
CAST_OBJ_NOTNULL(vps, req->privs, VRT_PRIVS_MAGIC);
- return (vrt_priv_dynamic(req->ws, vps, (uintptr_t)vmod_id));
+ return (vrt_priv_dynamic(req->ws, vps, id, (uintptr_t)vmod_id));
}
/*--------------------------------------------------------------------
diff --git a/bin/varnishtest/tests/r03003.vtc b/bin/varnishtest/tests/r03003.vtc
new file mode 100644
index 000000000..a95836178
--- /dev/null
+++ b/bin/varnishtest/tests/r03003.vtc
@@ -0,0 +1,23 @@
+varnishtest "Test PRIV_TOP vs PRIV_TASK"
+
+varnish v1 -vcl {
+ import debug;
+
+ backend bad { .host = "${bad_backend}"; }
+
+ sub vcl_recv {
+ return (synth(200));
+ }
+
+ sub vcl_synth {
+ set resp.http.priv_task = debug.test_priv_task("task");
+ set resp.http.priv_top = debug.test_priv_top("top");
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.http.priv_task == task
+ expect resp.http.priv_top == top
+} -run
More information about the varnish-commit
mailing list