[master] ab0214c80 Move HSH_Cancel() to VDP_Close() to bring it under transport control
Nils Goroll
nils.goroll at uplex.de
Mon May 15 13:26:06 UTC 2023
commit ab0214c8015a1694996e54e4df101816953963db
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Wed May 10 13:07:40 2023 +0200
Move HSH_Cancel() to VDP_Close() to bring it under transport control
Transports should be free to keep a reference on the object to be
delivered until after their transport function returns, but
HSH_Cancel() in cnt_transmit() prevented the object from being of any
use for the case that it is final (pass/hfm/hfp).
We solve this by moving the HSH_Cancel() close to VDP_Close, which
also makes sense from the perspective of the VDP design: Until the VDP
close, filters could still reference object data.
HSH_Cancel() needs the objcore, which could be reachable also via
vdc->req. But that member is unset in VDP_DeliverObj(), presumably to
make it clear that VDP .bytes callbacks should not access request
data. Thus, we pass it as a new argument to VDP_Close() as well as any
boc being held by the caller.
The objcore can also be NULL for the case where a transport generates
the body without holding an objcore at all.
diff --git a/bin/varnishd/cache/cache_deliver_proc.c b/bin/varnishd/cache/cache_deliver_proc.c
index 44507eb5a..a6d9832ed 100644
--- a/bin/varnishd/cache/cache_deliver_proc.c
+++ b/bin/varnishd/cache/cache_deliver_proc.c
@@ -33,6 +33,7 @@
#include "cache_varnishd.h"
#include "cache_filter.h"
+#include "cache_objhead.h"
void
VDP_Panic(struct vsb *vsb, const struct vdp_ctx *vdc)
@@ -182,12 +183,16 @@ VDP_Push(VRT_CTX, struct vdp_ctx *vdc, struct ws *ws, const struct vdp *vdp,
}
uint64_t
-VDP_Close(struct vdp_ctx *vdc)
+VDP_Close(struct vdp_ctx *vdc, struct objcore *oc, struct boc *boc)
{
struct vdp_entry *vdpe;
uint64_t rv = 0;
CHECK_OBJ_NOTNULL(vdc, VDP_CTX_MAGIC);
+ CHECK_OBJ_NOTNULL(vdc->wrk, WORKER_MAGIC);
+ CHECK_OBJ_ORNULL(oc, OBJCORE_MAGIC);
+ CHECK_OBJ_ORNULL(boc, BOC_MAGIC);
+
while (!VTAILQ_EMPTY(&vdc->vdp)) {
vdpe = VTAILQ_FIRST(&vdc->vdp);
rv = vdpe->bytes_in;
@@ -209,6 +214,8 @@ VDP_Close(struct vdp_ctx *vdc)
assert(vdpe->end == VDP_END);
#endif
}
+ if (oc != NULL)
+ HSH_Cancel(vdc->wrk, oc, boc);
return (rv);
}
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 56f4b82d2..762e42ae2 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -924,7 +924,7 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
if (i && req->doclose == SC_NULL)
req->doclose = SC_REM_CLOSE;
- req->acct.resp_bodybytes += VDP_Close(req->vdc);
+ req->acct.resp_bodybytes += VDP_Close(req->vdc, req->objcore, boc);
if (i && !ecx->incl_cont) {
req->top->topreq->vdc->retval = -1;
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index fabddba5b..789cea540 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -488,8 +488,6 @@ cnt_transmit(struct worker *wrk, struct req *req)
VSLb_ts_req(req, "Resp", W_TIM_real(wrk));
- HSH_Cancel(wrk, req->objcore, boc);
-
if (req->doclose == SC_NULL && (req->objcore->flags & OC_F_FAILED)) {
/* The object we delivered failed due to a streaming error.
* Fail the request. */
@@ -497,7 +495,7 @@ cnt_transmit(struct worker *wrk, struct req *req)
}
if (req->doclose != SC_NULL)
- req->acct.resp_bodybytes += VDP_Close(req->vdc);
+ req->acct.resp_bodybytes += VDP_Close(req->vdc, req->objcore, boc);
if (boc != NULL)
HSH_DerefBoc(wrk, req->objcore);
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 08763874c..b534eb174 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -191,7 +191,7 @@ void VDI_Init(void);
/* cache_deliver_proc.c */
void VDP_Init(struct vdp_ctx *vdx, struct worker *wrk, struct vsl_log *vsl,
struct req *req);
-uint64_t VDP_Close(struct vdp_ctx *);
+uint64_t VDP_Close(struct vdp_ctx *, struct objcore *, struct boc *);
void VDP_Panic(struct vsb *vsb, const struct vdp_ctx *vdc);
int VDP_Push(VRT_CTX, struct vdp_ctx *, struct ws *, const struct vdp *,
void *priv);
diff --git a/bin/varnishd/http1/cache_http1_deliver.c b/bin/varnishd/http1/cache_http1_deliver.c
index 00331091e..86a2dfd0e 100644
--- a/bin/varnishd/http1/cache_http1_deliver.c
+++ b/bin/varnishd/http1/cache_http1_deliver.c
@@ -163,7 +163,7 @@ V1D_Deliver(struct req *req, struct boc *boc, int sendbody)
AZ(req->wrk->v1l);
req->acct.resp_hdrbytes += hdrbytes;
- req->acct.resp_bodybytes += VDP_Close(req->vdc);
+ req->acct.resp_bodybytes += VDP_Close(req->vdc, req->objcore, boc);
if (sc == SC_NULL && err && req->sp->fd >= 0)
sc = SC_REM_CLOSE;
diff --git a/bin/varnishd/http2/cache_http2_deliver.c b/bin/varnishd/http2/cache_http2_deliver.c
index a19e936d5..632d999ef 100644
--- a/bin/varnishd/http2/cache_http2_deliver.c
+++ b/bin/varnishd/http2/cache_http2_deliver.c
@@ -351,5 +351,5 @@ h2_deliver(struct req *req, struct boc *boc, int sendbody)
}
AZ(req->wrk->v1l);
- req->acct.resp_bodybytes += VDP_Close(req->vdc);
+ req->acct.resp_bodybytes += VDP_Close(req->vdc, req->objcore, boc);
}
More information about the varnish-commit
mailing list