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

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

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

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

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