diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c index f4d3810a27..770a4afc33 100644 --- a/bin/varnishd/cache/cache_req_fsm.c +++ b/bin/varnishd/cache/cache_req_fsm.c @@ -65,7 +65,7 @@ REQ_STEP(fetch, FETCH, static) \ REQ_STEP(deliver, DELIVER, static) \ REQ_STEP(vclfail, VCLFAIL, static) \ - REQ_STEP(synth, SYNTH, static) \ + REQ_STEP(synth, SYNTH, ) \ REQ_STEP(transmit, TRANSMIT, static) \ REQ_STEP(finish, FINISH, static) @@ -341,7 +341,7 @@ cnt_synth(struct worker *wrk, struct req *req) VSLb_ts_req(req, "Process", W_TIM_real(wrk)); - while (wrk->vpi->handling == VCL_RET_FAIL) { + while (req->req_reset || wrk->vpi->handling == VCL_RET_FAIL) { if (req->esi_level > 0) { wrk->vpi->handling = VCL_RET_DELIVER; break; @@ -596,9 +596,9 @@ cnt_lookup(struct worker *wrk, struct req *req) if (lr == HSH_BUSY) { /* * We lost the session to a busy object, disembark the - * worker thread. We return to STP_LOOKUP when the busy - * object has been unbusied, and still have the objhead - * around to restart the lookup with. + * worker thread. We return to R_STP_LOOKUP, with the + * object still around when it has been unbusied, to + * restart the lookup with. */ return (REQ_FSM_DISEMBARK); } diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h index a2edf20833..ed12238f03 100644 --- a/bin/varnishd/cache/cache_varnishd.h +++ b/bin/varnishd/cache/cache_varnishd.h @@ -64,6 +64,7 @@ struct req_step { extern const struct req_step R_STP_TRANSPORT[1]; extern const struct req_step R_STP_RECV[1]; +extern const struct req_step R_STP_SYNTH[1]; struct vxid_pool { uint64_t next; diff --git a/bin/varnishd/cache/cache_vrt_vcl.c b/bin/varnishd/cache/cache_vrt_vcl.c index b4e77d1581..325ed2d134 100644 --- a/bin/varnishd/cache/cache_vrt_vcl.c +++ b/bin/varnishd/cache/cache_vrt_vcl.c @@ -560,13 +560,16 @@ req_poll(struct worker *wrk, struct req *req) { struct req *top; - /* NB: Since a fail transition leads to vcl_synth, the request may be - * short-circuited twice. + /* If the req_reset flag was raised without taking the emulated + * return(fail) transition, there must be a good reason for that. + * One is that sub vcl_recv is always followed by a call to sub + * vcl_hash to always compute a cache key. The other one is that + * we always want to run sub vcl_synth for return(fail) even if + * we won't deliver the response, and maybe sub vcl_synth needs + * the cache key. There may be other good reasons to proceed. */ - if (req->req_reset) { - wrk->vpi->handling = VCL_RET_FAIL; - return (-1); - } + if (req->req_reset) + return (0); top = req->top->topreq; CHECK_OBJ_NOTNULL(top, REQ_MAGIC); @@ -581,8 +584,12 @@ req_poll(struct worker *wrk, struct req *req) VSLb_ts_req(req, "Reset", W_TIM_real(wrk)); wrk->stats->req_reset++; - wrk->vpi->handling = VCL_RET_FAIL; req->req_reset = 1; + + if (req->req_step == R_STP_SYNTH) + return (0); + + wrk->vpi->handling = VCL_RET_FAIL; return (-1); } diff --git a/bin/varnishtest/tests/t02025.vtc b/bin/varnishtest/tests/t02025.vtc index 39f987a068..58272056f2 100644 --- a/bin/varnishtest/tests/t02025.vtc +++ b/bin/varnishtest/tests/t02025.vtc @@ -18,6 +18,12 @@ varnish v1 -vcl { sub vcl_miss { vtc.panic("unreachable"); } + + sub vcl_synth { + if (resp.status == 408) { + set resp.http.gone = "bye"; + } + } } -start logexpect l1 -v v1 -g raw -i Debug { @@ -51,7 +57,8 @@ varnish v1 -expect req_reset == 1 # is interpreted as before a second elapsed. Session VXIDs showing up # numerous times become increasingly more suspicious. The format can of # course be extended to add anything else useful for data mining. -shell -expect "1000 ${localhost} 408" { +shell -expect "1000 ${localhost} 408 bye" { varnishncsa -n ${v1_name} -d \ - -q 'Timestamp:Reset[2] < 1.0' -F '%{VSL:Begin[2]}x %h %s' + -q 'Timestamp:Reset[2] < 1.0' \ + -F '%{VSL:Begin[2]}x %h %s %{gone}o' }