[master] a280f82b1 Resolve race on waitinglist rushing between HSH_Unbusy and boc->state
Martin Blix Grydeland
martin at varnish-software.com
Thu May 2 12:45:10 UTC 2019
commit a280f82b18d8b736339370204f39141f31813d13
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date: Thu May 2 13:21:54 2019 +0200
Resolve race on waitinglist rushing between HSH_Unbusy and boc->state
This is the second attempt at a fix for this, the previous one was
reverted.
When an object is ready for delivery, HSH_Unbusy was called before calling
ObjSetState([BOS_STREAM|BOS_FINISHED]). The HSH_Unbusy() call does the
waitinglist rushing, but HSH_Lookup() wanted to look at the boc->state and
if BOS_STREAM had been reached. This could cause requests woken to find
that the stream state still hadn't been reached (ObjSetState still hadn't
executed), and go back on the waitinglist.
To fix this, this patch reverts commit
0375791cac1f2333ab54932ba1d2025261082fab, and goes back to considering
OC_F_BUSY as the gate keeper for HSH_Lookup. This eliminates the race,
because HSH_Unbusy and HSH_Lookup then uses the same mutex.
That change opens up the possiblity that req code after HSH_Lookup() sees
an object that has not yet reached BOS_STREAM. To counter this, once a boc
reference is found during delivery, an ObjWaitState(BOS_STREAM) is done
before continuing.
Lastly, an ObjSetState(BOS_PREP_STREAM) is added just before HSH_Unbusy()
in vbf_stp_error() and vbf_stp_fetchend() for the case where
do_stream==false. This makes the order of events consistent throughout.
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index b3304bedb..eb25de6e9 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -593,6 +593,7 @@ vbf_stp_fetchend(struct worker *wrk, struct busyobj *bo)
assert(bo->fetch_objcore->boc->state == BOS_STREAM);
else {
assert(bo->fetch_objcore->boc->state == BOS_REQ_DONE);
+ ObjSetState(wrk, bo->fetch_objcore, BOS_PREP_STREAM);
HSH_Unbusy(wrk, bo->fetch_objcore);
}
@@ -790,6 +791,7 @@ 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_PREP_STREAM);
HSH_Unbusy(wrk, bo->fetch_objcore);
if (stale != NULL && bo->fetch_objcore->ttl > 0)
HSH_Kill(stale);
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index ead55932c..8796e2adf 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->boc != NULL && oc->boc->state < BOS_STREAM) {
+ if (oc->flags & OC_F_BUSY) {
if (req->hash_ignore_busy)
continue;
- if (oc->boc->vary != NULL &&
+ if (oc->boc && oc->boc->vary != NULL &&
!VRY_Match(req, oc->boc->vary))
continue;
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 5a7ef97e3..d4b343b46 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -353,6 +353,8 @@ cnt_transmit(struct worker *wrk, struct req *req)
/* Grab a ref to the bo if there is one (=streaming) */
boc = HSH_RefBoc(req->objcore);
+ if (boc && boc->state < BOS_STREAM)
+ ObjWaitState(req->objcore, BOS_STREAM);
clval = http_GetContentLength(req->resp);
/* RFC 7230, 3.3.3 */
status = http_GetStatus(req->resp);
More information about the varnish-commit
mailing list