[master] 8b41c8c01 condfetch: Demote to fetch for dying stale_oc

[Nils Goroll]

commit 8b41c8c016d06be65f3b88365efb277e2d6def62
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Mar 12 18:12:33 2024 +0100

    condfetch: Demote to fetch for dying stale_oc
    
    When it is time to decide between making a bereq for a regular or conditional
    fetch, we need to ensure that the stale object is still valid. There is a window
    between the lookup and the begining of a fetch task during which the object
    could have been invalidated, in particular if the fetch task was queued.
    
    Catching it early allows to proceed with a regular fetch.
    
    Comitter edit: Minor wording change above, because the mkbereq step is not
    entered for retries.
    
    Ref #4399

diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 55374cf09..e6fb30cc2 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -272,7 +272,7 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo)
 	}
 	http_ForceField(bo->bereq0, HTTP_HDR_PROTO, "HTTP/1.1");
 
-	if (bo->stale_oc != NULL &&
+	if (bo->stale_oc != NULL && !(bo->stale_oc->flags & OC_F_DYING) &&
 	    ObjCheckFlag(bo->wrk, bo->stale_oc, OF_IMSCAND) &&
 	    (bo->stale_oc->boc != NULL || ObjGetLen(wrk, bo->stale_oc) != 0)) {
 		AZ(bo->stale_oc->flags & (OC_F_HFM|OC_F_PRIVATE));
diff --git a/bin/varnishtest/tests/c00130.vtc b/bin/varnishtest/tests/c00130.vtc
new file mode 100644
index 000000000..c75f24621
--- /dev/null
+++ b/bin/varnishtest/tests/c00130.vtc
@@ -0,0 +1,59 @@
+varnishtest "Demote condfetch to fetch on invalidated stale object"
+
+barrier b1 sock 2
+barrier b2 sock 2
+
+server s1 {
+	rxreq
+	txresp -hdr {ETag: "foo"} -body corrupted
+
+	rxreq
+	expect req.http.If-None-Match == <undef>
+	txresp -hdr {ETag: "foo"} -body valid
+} -start
+
+varnish v1 -vcl+backend {
+	import vtc;
+	sub vcl_recv {
+		if (req.method == "PURGE") {
+			return (purge);
+		}
+	}
+	sub vcl_miss {
+		if (req.http.sync) {
+			vtc.barrier_sync("${b1_sock}");
+			vtc.barrier_sync("${b2_sock}");
+		}
+	}
+	sub vcl_backend_response {
+		set beresp.ttl = 1ms;
+		set beresp.grace = 0s;
+		set beresp.keep = 10s;
+	}
+	sub vcl_deliver {
+		set resp.http.obj-hits = obj.hits;
+	}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.body == corrupted
+	expect resp.http.obj-hits == 0
+
+	delay 0.1
+
+	txreq -hdr "sync: true"
+	rxresp
+	expect resp.body == valid
+	expect resp.http.obj-hits == 0
+} -start
+
+barrier b1 sync
+client c2 {
+	txreq -req PURGE
+	rxresp
+} -run
+barrier b2 sync
+
+client c1 -wait