fetch: Close data race between BOC state and objcore flags

bin/varnishd/cache/cache_esi_deliver.c

@@ -911,7 +911,7 @@ ved_deliver(struct req *req, int wantbody)
 
 		/* OA_GZIPBITS are not valid until BOS_FINISHED */
 		if (req->boc != NULL)
-			ObjWaitState(req->objcore, BOS_FINISHED);
+			(void)ObjWaitState(req->objcore, BOS_FINISHED);
 
 		if (req->objcore->flags & OC_F_FAILED) {
 			/* No way of signalling errors in the middle of

bin/varnishd/cache/cache_fetch.c

@@ -886,8 +886,7 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
 		VSLb(bo->vsl, SLT_Notice,
 		    "vsl: Conditional fetch wait for streaming object");
 		/* XXX: We should have a VCL controlled timeout here */
-		ObjWaitState(stale_oc, BOS_FINISHED);
-		stale_state = stale_boc->state;
+		stale_state = ObjWaitState(stale_oc, BOS_FINISHED);
 		HSH_DerefBoc(bo->wrk, stale_oc);
 		stale_boc = NULL;
 		if (stale_state != BOS_FINISHED) {
@@ -1172,6 +1171,7 @@ void
 VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc,
     struct objcore *oldoc, enum vbf_fetch_mode_e mode)
 {
+	enum boc_state_e state;
 	struct boc *boc;
 	struct busyobj *bo;
 	enum task_prio prio;
@@ -1257,12 +1257,12 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc,
 		THR_SetBusyobj(NULL);
 		bo = NULL; /* ref transferred to fetch thread */
 		if (mode == VBF_BACKGROUND) {
-			ObjWaitState(oc, BOS_REQ_DONE);
+			(void)ObjWaitState(oc, BOS_REQ_DONE);
 			(void)VRB_Ignore(req);
 		} else {
-			ObjWaitState(oc, BOS_STREAM);
+			state = ObjWaitState(oc, BOS_STREAM);
 			AZ(oc->flags & OC_F_BUSY);
-			if (oc->boc->state == BOS_FAILED)
+			if (state == BOS_FAILED)
 				AN(oc->flags & OC_F_FAILED);
 		}
 	}

bin/varnishd/cache/cache_hash.c

@@ -990,7 +990,7 @@ HSH_Cancel(struct worker *wrk, struct objcore *oc, struct boc *boc)
 
 	if (boc != NULL) {
 		hsh_cancel(oc);
-		ObjWaitState(oc, BOS_FINISHED);
+		(void)ObjWaitState(oc, BOS_FINISHED);
 	}
 
 	if (bocref != NULL)

bin/varnishd/cache/cache_obj.c

@@ -524,9 +524,10 @@ ObjSetState(struct worker *wrk, struct objcore *oc, enum boc_state_e next,
 /*====================================================================
  */
 
-void
+enum boc_state_e
 ObjWaitState(const struct objcore *oc, enum boc_state_e want)
 {
+	enum boc_state_e got;
 
 	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
 	CHECK_OBJ_NOTNULL(oc->boc, BOC_MAGIC);
@@ -540,7 +541,10 @@ ObjWaitState(const struct objcore *oc, enum boc_state_e want)
 			break;
 		(void)Lck_CondWait(&oc->boc->cond, &oc->boc->mtx);
 	}
+	got = oc->boc->state;
 	Lck_Unlock(&oc->boc->mtx);
+
+	return (got);
 }
 
 /*====================================================================

bin/varnishd/cache/cache_req_fsm.c

@@ -448,7 +448,7 @@ cnt_transmit(struct worker *wrk, struct req *req)
 	/* Grab a ref to the bo if there is one (=streaming) */
 	req->boc = HSH_RefBoc(req->objcore);
 	if (req->boc && req->boc->state < BOS_STREAM)
-		ObjWaitState(req->objcore, BOS_STREAM);
+		(void)ObjWaitState(req->objcore, BOS_STREAM);
 	clval = http_GetContentLength(req->resp);
 	/* RFC 7230, 3.3.3 */
 	status = http_GetStatus(req->resp);

bin/varnishd/cache/cache_varnishd.h

@@ -346,7 +346,7 @@ uint64_t ObjWaitExtend(const struct worker *, const struct objcore *,
     uint64_t l, enum boc_state_e *statep);
 void ObjSetState(struct worker *, struct objcore *, enum boc_state_e next,
     unsigned broadcast);
-void ObjWaitState(const struct objcore *, enum boc_state_e want);
+enum boc_state_e ObjWaitState(const struct objcore *, enum boc_state_e want);
 void ObjTouch(struct worker *, struct objcore *, vtim_real now);
 void ObjFreeObj(struct worker *, struct objcore *);
 void ObjSlim(struct worker *, struct objcore *);

tools/coccinelle/obj_wait_state_check.cocci

@@ -0,0 +1,39 @@
+/*
+ * Inspect the state returned by ObjWaitState().
+ */
+
+@capture_state@
+identifier func;
+expression oc;
+@@
+
+ func(...)
+ {
+ ...
+-ObjWaitState(oc,
++state = ObjWaitState(oc,
+ ...);
+ ...
+-oc->boc->state
++state
+ ...
+ }
+
+@declare_state@
+identifier capture_state.func;
+@@
+
+ func(...)
+ {
++enum boc_state_e state;
+ ...
+ }
+
+@ignore_state@
+statement stmt;
+@@
+
+<...stmt...>
+-ObjWaitState(
++(void)ObjWaitState(
+ ...);