SQUASHME: Stash stale_oc available during vcl_miss also

While VCL has no access to the stale_oc during vcl_miss, a vmod could reference
strings from it, so we also need to stash it until the end of the transaction.

This roughly doubles the number of pointers we need to keep around, but the
amount of additional memory on the workspace is still manageable. For the
default max_restarts = 4, it is 80 bytes on 64bit.

Author: nigoroll

bin/varnishd/cache/cache_req.c

@@ -84,11 +84,11 @@ ocstash_clear(struct worker *wrk, struct ocstash *stash)
 }
 
 void
-Req_StashObjcore(struct req *req)
+Req_StashObjcore(struct req *req, struct objcore **ocp)
 {
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
-	ocstash_push(req->stash, &req->objcore);
+	ocstash_push(req->stash, ocp);
 }
 
 /*--------------------------------------------------------------------*/
@@ -241,7 +241,8 @@ Req_New(struct sess *sp, const struct req *preq)
 
 	req->max_restarts = cache_param->max_restarts;
 	assert(req->max_restarts + 1 <= UINT16_MAX);
-	l = req->max_restarts + 1;
+	// each restart may ref one stale_oc and one oc
+	l = (2 * req->max_restarts) + 1;
 	sz = SIZEOF_FLEX_OBJ(req->stash, ocs, l);
 	req->stash = (void*)p;
 	req->stash->magic = OCSTASH_MAGIC;

bin/varnishd/cache/cache_req_fsm.c

@@ -240,7 +240,7 @@ cnt_deliver(struct worker *wrk, struct req *req)
 
 	if (wrk->vpi->handling != VCL_RET_DELIVER) {
 		HSH_Cancel(wrk, req->objcore, NULL);
-		Req_StashObjcore(req);
+		Req_StashObjcore(req, &req->objcore);
 		http_Teardown(req->resp);
 
 		switch (wrk->vpi->handling) {
@@ -684,7 +684,7 @@ cnt_lookup(struct worker *wrk, struct req *req)
 		WRONG("Illegal return from vcl_hit{}");
 	}
 
-	Req_StashObjcore(req);
+	Req_StashObjcore(req, &req->objcore);
 
 	if (busy != NULL) {
 		(void)HSH_DerefObjCore(wrk, &busy, 0);
@@ -714,7 +714,7 @@ cnt_miss(struct worker *wrk, struct req *req)
 		wrk->stats->cache_miss++;
 		VBF_Fetch(wrk, req, req->objcore, req->stale_oc, VBF_NORMAL);
 		if (req->stale_oc != NULL)
-			(void)HSH_DerefObjCore(wrk, &req->stale_oc, 0);
+			Req_StashObjcore(req, &req->stale_oc);
 		req->req_step = R_STP_FETCH;
 		return (REQ_FSM_MORE);
 	case VCL_RET_FAIL:
@@ -734,7 +734,7 @@ cnt_miss(struct worker *wrk, struct req *req)
 	}
 	VRY_Clear(req);
 	if (req->stale_oc != NULL)
-		(void)HSH_DerefObjCore(wrk, &req->stale_oc, 0);
+		Req_StashObjcore(req, &req->stale_oc);
 	AZ(HSH_DerefObjCore(wrk, &req->objcore, 1));
 	return (REQ_FSM_MORE);
 }

bin/varnishd/cache/cache_varnishd.h

@@ -419,7 +419,7 @@ void Req_Fail(struct req *req, stream_close_t reason);
 void Req_AcctLogCharge(struct VSC_main_wrk *, struct req *);
 void Req_LogHit(struct worker *, struct req *, struct objcore *, intmax_t);
 const char *Req_LogStart(const struct worker *, struct req *);
-void Req_StashObjcore(struct req *);
+void Req_StashObjcore(struct req *, struct objcore **);
 
 /* cache_req_body.c */
 int VRB_Ignore(struct req *);

bin/varnishtest/tests/r02219.vtc

@@ -11,6 +11,7 @@ server s1 {
 
 varnish v1 -arg "-p workspace_client=9k" \
 	   -arg "-p vsl_buffer=4k" \
+	   -arg "-p max_restarts=2" \
 	   -proto PROXY \
 	   -vcl+backend {
 	import std;

include/tbl/params.h

@@ -786,7 +786,7 @@ PARAM_SIMPLE(
 	/* name */	max_restarts,
 	/* type */	uint,
 	/* min */	"0",
-	/* max */	"65534",	// (1<<16)-2 #4269
+	/* max */	"32766",	// (1<<15)-2 #4269
 	/* def */	"4",
 	/* units */	"restarts",
 	/* descr */