[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