[6.0] 98391cb8e condfetch: Handle failed stale_oc without a boc
Reza Naghibi
reza at naghibi.com
Tue Apr 20 18:24:04 UTC 2021
commit 98391cb8e673ef646156f628aec599dc8efb619e
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Mon Mar 22 12:16:39 2021 +0100
condfetch: Handle failed stale_oc without a boc
The assertion that the stale objcore of a conditional fetch cannot be
failed unless it was streaming is incorrect. Between the moment when
we grab the stale objcore in HSH_Lookup and the moment we try to use
it after vcl_backend_response, the backend fetch may have completed or
failed.
Instead, we need to treat an ongoing fetch and a failed fetch as
separate checks since the latter may happen with or without a boc.
Conflicts:
bin/varnishd/cache/cache_fetch.c
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 63d3564fe..034424c6a 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -799,13 +799,18 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
HSH_DerefBoc(bo->wrk, bo->stale_oc);
stale_boc = NULL;
if (stale_state != BOS_FINISHED) {
- (void)VFP_Error(bo->vfc, "Template object failed");
- VDI_Finish(bo->wrk, bo);
- wrk->stats->fetch_failed++;
- return (F_STP_FAIL);
+ assert(stale_state == BOS_FAILED);
+ AN(bo->stale_oc->flags & OC_F_FAILED);
}
}
- AZ(bo->stale_oc->flags & OC_F_FAILED);
+
+ AZ(stale_boc);
+ if (bo->stale_oc->flags & OC_F_FAILED) {
+ (void)VFP_Error(bo->vfc, "Template object failed");
+ vbf_cleanup(bo);
+ wrk->stats->fetch_failed++;
+ return (F_STP_FAIL);
+ }
if (vbf_beresp2obj(bo)) {
(void)VFP_Error(bo->vfc, "Could not get storage");
diff --git a/bin/varnishtest/tests/c00105.vtc b/bin/varnishtest/tests/c00105.vtc
new file mode 100644
index 000000000..a5588b550
--- /dev/null
+++ b/bin/varnishtest/tests/c00105.vtc
@@ -0,0 +1,60 @@
+varnishtest "Failed post-streaming revalidation"
+
+barrier b1 cond 2
+barrier b2 sock 2
+barrier b3 sock 2
+
+server s1 {
+ rxreq
+ txresp -nolen -hdr {Etag: "abc"} -hdr "Content-Length: 100"
+ barrier b1 sync
+ barrier b2 sync
+} -start
+
+server s2 {
+ rxreq
+ expect req.http.If-None-Match == {"abc"}
+ txresp -status 304 -nolen -hdr {Etag: "abc"} -hdr "Content-Length: 100"
+} -start
+
+varnish v1 -vcl+backend {
+ import directors;
+ import vtc;
+
+ sub vcl_recv {
+ if (req.http.backend == "s2") {
+ set req.backend_hint = s2;
+ }
+ }
+
+ sub vcl_backend_response {
+ if (beresp.was_304) {
+ vtc.barrier_sync("${b2_sock}");
+ vtc.barrier_sync("${b3_sock}");
+ }
+ set beresp.ttl = 1ms;
+ }
+} -start
+
+client c1 {
+ txreq -hdr "backend: s1"
+ rxresphdrs
+ expect resp.status == 200
+ expect_close
+} -start
+
+barrier b1 sync
+
+# ensure stale_oc
+delay 0.01
+
+client c2 {
+ txreq -hdr "backend: s2"
+ rxresphdrs
+ expect resp.status == 200
+ expect_close
+} -start
+
+client c1 -wait
+barrier b3 sync
+client c2 -wait
More information about the varnish-commit
mailing list