[master] 7046979e6 Revert "Resolve race on waitinglist rushing between OC_F_BUSY and boc->state"

[Martin Blix Grydeland]

commit 7046979e6e29461b075a7d3b35c51d1b06f98dc4
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date:   Fri Apr 26 15:32:16 2019 +0200

    Revert "Resolve race on waitinglist rushing between OC_F_BUSY and boc->state"
    
    This reverts commit 4130055c457903e7f904fa56c0db58d7e7467dc1.

diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 63738e1fe..b3304bedb 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -562,8 +562,9 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 	assert(bo->fetch_objcore->boc->state == BOS_REQ_DONE);
 
 	if (bo->do_stream) {
-		ObjSetState(wrk, bo->fetch_objcore, BOS_STREAM);
+		ObjSetState(wrk, bo->fetch_objcore, BOS_PREP_STREAM);
 		HSH_Unbusy(wrk, bo->fetch_objcore);
+		ObjSetState(wrk, bo->fetch_objcore, BOS_STREAM);
 	}
 
 	VSLb(bo->vsl, SLT_Fetch_Body, "%u %s %s",
@@ -588,20 +589,18 @@ vbf_stp_fetchend(struct worker *wrk, struct busyobj *bo)
 	AZ(ObjSetU64(wrk, bo->fetch_objcore, OA_LEN,
 	    bo->fetch_objcore->boc->len_so_far));
 
-	/* Recycle the backend connection before setting BOS_FINISHED to
-	   give predictable backend reuse behavior for varnishtest */
-	VDI_Finish(bo);
-
 	if (bo->do_stream)
 		assert(bo->fetch_objcore->boc->state == BOS_STREAM);
-	else
+	else {
 		assert(bo->fetch_objcore->boc->state == BOS_REQ_DONE);
-	ObjSetState(wrk, bo->fetch_objcore, BOS_FINISHED);
-
-	if (!bo->do_stream)
 		HSH_Unbusy(wrk, bo->fetch_objcore);
-	AZ(bo->fetch_objcore->flags & OC_F_BUSY);
+	}
 
+	/* Recycle the backend connection before setting BOS_FINISHED to
+	   give predictable backend reuse behavior for varnishtest */
+	VDI_Finish(bo);
+
+	ObjSetState(wrk, bo->fetch_objcore, BOS_FINISHED);
 	VSLb_ts_busyobj(bo, "BerespBody", W_TIM_real(wrk));
 	if (bo->stale_oc != NULL)
 		HSH_Kill(bo->stale_oc);
@@ -655,8 +654,9 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
 	AZ(ObjCopyAttr(bo->wrk, bo->fetch_objcore, bo->stale_oc, OA_GZIPBITS));
 
 	if (bo->do_stream) {
-		ObjSetState(wrk, bo->fetch_objcore, BOS_STREAM);
+		ObjSetState(wrk, bo->fetch_objcore, BOS_PREP_STREAM);
 		HSH_Unbusy(wrk, bo->fetch_objcore);
+		ObjSetState(wrk, bo->fetch_objcore, BOS_STREAM);
 	}
 
 	if (ObjIterate(wrk, bo->stale_oc, bo, vbf_objiterator, 0))
@@ -790,10 +790,10 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 	}
 	AZ(ObjSetU64(wrk, bo->fetch_objcore, OA_LEN, o));
 	VSB_destroy(&synth_body);
-	ObjSetState(wrk, bo->fetch_objcore, BOS_FINISHED);
 	HSH_Unbusy(wrk, bo->fetch_objcore);
 	if (stale != NULL && bo->fetch_objcore->ttl > 0)
 		HSH_Kill(stale);
+	ObjSetState(wrk, bo->fetch_objcore, BOS_FINISHED);
 	return (F_STP_DONE);
 }
 
@@ -808,10 +808,10 @@ vbf_stp_fail(struct worker *wrk, const struct busyobj *bo)
 	CHECK_OBJ_NOTNULL(bo->fetch_objcore, OBJCORE_MAGIC);
 
 	assert(bo->fetch_objcore->boc->state < BOS_FINISHED);
-	ObjSetState(wrk, bo->fetch_objcore, BOS_FAILED);
 	HSH_Fail(bo->fetch_objcore);
 	if (!(bo->fetch_objcore->flags & OC_F_BUSY))
 		HSH_Kill(bo->fetch_objcore);
+	ObjSetState(wrk, bo->fetch_objcore, BOS_FAILED);
 	return (F_STP_DONE);
 }
 
@@ -982,6 +982,8 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc,
 			ObjWaitState(oc, BOS_STREAM);
 			if (oc->boc->state == BOS_FAILED) {
 				AN((oc->flags & OC_F_FAILED));
+			} else {
+				AZ(oc->flags & OC_F_BUSY);
 			}
 		}
 	}
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 66167df33..ead55932c 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -420,11 +420,11 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp)
 			continue;
 
 		CHECK_OBJ_ORNULL(oc->boc, BOC_MAGIC);
-		if (oc->flags & OC_F_BUSY) {
+		if (oc->boc != NULL && oc->boc->state < BOS_STREAM) {
 			if (req->hash_ignore_busy)
 				continue;
 
-			if (oc->boc != NULL && oc->boc->vary != NULL &&
+			if (oc->boc->vary != NULL &&
 			    !VRY_Match(req, oc->boc->vary))
 				continue;
 
diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c
index 89bb0497d..b533eefcf 100644
--- a/bin/varnishd/cache/cache_obj.c
+++ b/bin/varnishd/cache/cache_obj.c
@@ -267,6 +267,7 @@ ObjSetState(struct worker *wrk, const struct objcore *oc,
 	assert(next > oc->boc->state);
 
 	CHECK_OBJ_ORNULL(oc->stobj->stevedore, STEVEDORE_MAGIC);
+	assert(next != BOS_STREAM || oc->boc->state == BOS_PREP_STREAM);
 	assert(next != BOS_FINISHED || (oc->oa_present & (1 << OA_LEN)));
 
 	if (oc->stobj->stevedore != NULL) {
diff --git a/include/tbl/boc_state.h b/include/tbl/boc_state.h
index 751456ca6..a5c4a61ff 100644
--- a/include/tbl/boc_state.h
+++ b/include/tbl/boc_state.h
@@ -30,6 +30,7 @@
 
 BOC_STATE(INVALID,	invalid)	/* don't touch (yet) */
 BOC_STATE(REQ_DONE,	req_done)	/* bereq.* can be examined */
+BOC_STATE(PREP_STREAM,	prep_stream)	/* Prepare for streaming */
 BOC_STATE(STREAM,	stream)		/* beresp.* can be examined */
 BOC_STATE(FINISHED,	finished)	/* object is complete */
 BOC_STATE(FAILED,	failed)		/* something went wrong */