Mailing List Archive

commit 62b0192a7df674b50017e054f6b7d2daa40cfaeb
Author: Nils Goroll <nils.goroll@uplex.de>
Date: Wed Oct 2 16:49:28 2019 +0200

fix bereq rollback

by properly cleaning up the busyobj

Also move the relevant code from cache_vrt.c to cache_fetch.c

As we fini the director during cleanup, we now also need to handle the
backend connection gone missing in vbf_stp_fetch(). The hypothetical
alternative would be to not fini the director, but I believe this is not
safe in case it also used some workspace.

Fixes #3009

Conflicts:
bin/varnishd/cache/cache_fetch.c
bin/varnishd/cache/cache_varnishd.h
bin/varnishd/cache/cache_vrt.c

diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 0c0d79c9d..47cc8fd79 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -99,6 +99,18 @@ vbf_cleanup(struct busyobj *bo)
VDI_Finish(bo->wrk, bo);
}

+void Bereq_Rollback(struct busyobj *bo)
+{
+ CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+
+ vbf_cleanup(bo);
+ VCL_TaskLeave(bo->vcl, bo->privs);
+ VCL_TaskEnter(bo->vcl, bo->privs);
+ HTTP_Copy(bo->bereq, bo->bereq0);
+ WS_Reset(bo->bereq->ws, bo->ws_bo);
+ WS_Reset(bo->ws, bo->ws_bo);
+}
+
/*--------------------------------------------------------------------
* Turn the beresp into a obj
*/
@@ -404,7 +416,8 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)

if (wrk->handling == VCL_RET_ABANDON || wrk->handling == VCL_RET_FAIL ||
wrk->handling == VCL_RET_ERROR) {
- bo->htc->doclose = SC_RESP_CLOSE;
+ if (bo->htc)
+ bo->htc->doclose = SC_RESP_CLOSE;
vbf_cleanup(bo);
if (wrk->handling == VCL_RET_ERROR)
return (F_STP_ERROR);
@@ -413,7 +426,7 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
}

if (wrk->handling == VCL_RET_RETRY) {
- if (bo->htc->body_status != BS_NONE)
+ if (bo->htc && bo->htc->body_status != BS_NONE)
bo->htc->doclose = SC_RESP_CLOSE;
vbf_cleanup(bo);

@@ -596,6 +609,12 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)

assert(wrk->handling == VCL_RET_DELIVER);

+ if (bo->htc == NULL) {
+ (void)VFP_Error(bo->vfc, "No backend connection (rollback?)");
+ vbf_cleanup(bo);
+ return (F_STP_ERROR);
+ }
+
if (vbf_figure_out_vfp(bo)) {
(bo)->htc->doclose = SC_OVERLOAD;
vbf_cleanup(bo);
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 939a0fe17..4eb713fd3 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -202,6 +202,7 @@ enum vbf_fetch_mode_e {
};
void VBF_Fetch(struct worker *wrk, struct req *req,
struct objcore *oc, struct objcore *oldoc, enum vbf_fetch_mode_e);
+void Bereq_Rollback(struct busyobj *);

/* cache_fetch_proc.c */
void VFP_Init(void);
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index d0188492e..d103a0616 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -663,12 +663,7 @@ VRT_Rollback(VRT_CTX, VCL_HTTP hp)
Req_Rollback(ctx->req);
} else if (hp == ctx->http_bereq) {
CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
- // -> VBO_Rollback ?
- VCL_TaskLeave(ctx->bo->vcl, ctx->bo->privs);
- VCL_TaskEnter(ctx->bo->vcl, ctx->bo->privs);
- HTTP_Copy(ctx->bo->bereq, ctx->bo->bereq0);
- WS_Reset(ctx->bo->bereq->ws, ctx->bo->ws_bo);
- WS_Reset(ctx->bo->ws, ctx->bo->ws_bo);
+ Bereq_Rollback(ctx->bo);
} else
WRONG("VRT_Rollback 'hp' invalid");
}
diff --git a/bin/varnishtest/tests/r03009.vtc b/bin/varnishtest/tests/r03009.vtc
index 406a40d02..1bc44521f 100644
--- a/bin/varnishtest/tests/r03009.vtc
+++ b/bin/varnishtest/tests/r03009.vtc
@@ -28,6 +28,5 @@ varnish v1 -vcl+backend {
client c1 {
txreq
rxresp
- expect resp.status == 200
- expect resp.http.test == "1"
+ expect resp.status == 503
} -run
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit