From: Christopher Faulet Date: Wed, 21 Oct 2026 08:32:43 +0000 (+0000) Subject: BUG/MEDIUM: cache: do not create a second applet when two cache-use rules hit X-Git-Url: https://git.kaiwu.me/virtual_hosts.html?a=commitdiff_plain;h=a72de10226b265202f5820b48b7a52c99382a1b6;p=haproxy.git BUG/MEDIUM: cache: do not create a second applet when two cache-use rules hit A configuration with two "http-request cache-use" rules that may both find the requested object, for example two caches both fed by unconditional "cache-store" rules, or a cache-use rule in a defaults section that is repeated in the proxy, leaks memory and cache space on every request that hits both of them. Each such request leaks one appctx and its task (about 1kB on x86_64) and leaves the row of the first cache's entry retained forever. The memory grows with the number of requests until the process is restarted, which affects the system owner, and the rows retained this way can never be evicted anymore: once their entry expires, their blocks are lost and the cache progressively shrinks, until it cannot store anything. Any client may trigger this by simply requesting cached objects. With 2000 requests, "show pools" reports 2016 appctx used and "show cache" reports a refcount of 2000 on the entry, which stays like this after all the streams are gone. The cache-use action is not a final one. On a hit, it sets s->target to the cache applet and creates the applet on the back stream connector with sc_applet_create(), after retaining the entry and detaching its row. The next cache-use rule is still evaluated and, when it also hits, it calls sc_applet_create() again on the same stream connector. The new appctx takes the same sedesc, so the first one is not referenced anymore: it was woken up once, found no request in its input buffer and waits for one forever. Its release callback is never called, so the entry it retained is never released and its row never reattached to the avail list. Let's simply ignore the cache-use rule when the stream already has a target: the response will be produced by the applet that is already there. This is consistent with the stats applet that is only set up when s->target is NULL. An alternative would be to make the action final on a hit, but this would change the evaluation of the following rules for all users. Note that "use-service" has the same weakness, it creates its applet without checking whether one is already attached, so a cache hit followed by a matching use-service rule could leak the cache applet the same way. This one is left as is since use-service is a final action and mixing it with a cache hit is not a sensible configuration. This patch should be backported to all supported versions. In older versions, http_action_req_cache_use() may register the applet with a different function (si_register_handler(), stream_int_register_handler()) but the fix remains the same: return ACT_RET_CONT at the beginning of the function when s->target is set. --- diff --git a/reg-tests/cache/multiple_cache_use.vtc b/reg-tests/cache/multiple_cache_use.vtc new file mode 100644 index 000000000..a9e48a354 --- /dev/null +++ b/reg-tests/cache/multiple_cache_use.vtc @@ -0,0 +1,69 @@ +varnishtest "Two cache-use rules hitting the same request" + +# A hit on a first cache-use rule attaches the cache applet to the stream. A +# hit on a second cache-use rule must not create another applet, which would +# replace the first one and leak it with its retained cache row: the row's +# refcount would never drop back to zero. + +feature ignore_unknown_macro + +server s1 { + rxreq + txresp -hdr "Cache-Control: max-age=60" -bodylen 3000 +} -start + +haproxy h1 -conf { + global + .if feature(THREAD) + thread-groups 1 + .endif + + defaults + mode http + timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" + timeout client "${HAPROXY_TEST_TIMEOUT-5s}" + timeout server "${HAPROXY_TEST_TIMEOUT-5s}" + + frontend fe + bind "fd@${fe}" + http-request cache-use c1 + http-request cache-use c2 + http-response cache-store c1 + http-response cache-store c2 + http-response set-header X-Cache-Hit %[res.cache_hit] + default_backend be + + backend be + server s1 ${s1_addr}:${s1_port} + + cache c1 + total-max-size 4 + + cache c2 + total-max-size 4 +} -start + +client c1 -connect ${h1_fe_sock} { + txreq -url "/a" + rxresp + expect resp.status == 200 + expect resp.bodylen == 3000 + expect resp.http.X-Cache-Hit == 0 +} -run + +# both caches hold the object now, every request hits both rules +client c2 -connect ${h1_fe_sock} -repeat 3 { + txreq -url "/a" + rxresp + expect resp.status == 200 + expect resp.bodylen == 3000 + expect resp.http.X-Cache-Hit == 1 +} -run + +delay 0.2 + +# the applets were all released, so the row of c1 is not retained anymore +haproxy h1 -cli { + send "show cache" + expect ~ "c1 .*\n.*type:full size:[0-9]+ \\([0-9]+ blocks\\), refcount:0," +} diff --git a/src/cache.c b/src/cache.c index 3e3a2119e..e11b13af8 100644 --- a/src/cache.c +++ b/src/cache.c @@ -2661,6 +2661,13 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p struct cache_tree *cache_tree = NULL; + /* The response is already produced by an applet, e.g. after a hit on + * a previous cache-use rule. Creating another one would replace it on + * the back stream connector, leaking it with its retained cache row. + */ + if (s->target) + return ACT_RET_CONT; + /* Ignore cache for HTTP/1.0 requests and for requests other than GET * and HEAD */ if (!(txn->req.flags & HTTP_MSGF_VER_11) ||