[master] ab0214c80 Move HSH_Cancel() to VDP_Close() to bring it under transport control

[Nils Goroll]

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);
 }