BUG/MEDIUM: cache: fix a refcount leak for missed secondary entries

When a primary cache hit has a Vary secondary_key_signature, the code calls
retain_entry() and shctx_row_detach() before performing the secondary lookup.
If get_secondary_entry() returns NULL (no stored variant matches), res is set
to NULL and the function falls through to return ACT_RET_CONT without calling
release_entry() or shctx_row_reattach(). Each such request leaks one refcount
and pins one shctx row permanently, eventually exhausting the cache if this
happens to all objects. This is visible when requesting a secondary key
covered by vary for an object that is already stored without that key.
"show cache" then shows the object's refcount increasing after each request.

In order to fix this we must do like when no secondary key could be built
and release everything. We only reattach to the row if we previously
detached.

The issue was introduced in 2.4 with commit 1785f3dd9 ("MEDIUM: cache: Add
the Vary header support"). The code changed a bit in 2.9 with commit
48f81ec09 ("MAJOR: cache: Delay cache entry delete in reserve_hot function"),
so in order to backport to 2.8 and older, the patch will have to be manually
applied (no test on detached).

diff --git a/src/cache.c b/src/cache.c
index 0c27eec1eb49e7e1e948f69f5c68807360dc242d..75ccc3c4df0186125d93da72e339408df85bd0c1 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -2180,7 +2180,17 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p
                                sec_entry = get_secondary_entry(cache_tree, res,
                                                                s->txn.http->cache_secondary_hash,
                                                                0);
-                               if (sec_entry && sec_entry != res) {
+                               if (!sec_entry) {
+                                       /* Secondary key miss: release the retained primary entry
+                                        * and reattach the detached row before returning.
+                                        */
+                                       release_entry(cache_tree, res, 0);
+                                       shctx_wrlock(shctx);
+                                       if (detached)
+                                               shctx_row_reattach(shctx, entry_block);
+                                       shctx_wrunlock(shctx);
+                               }
+                               else if (sec_entry != res) {
                                        /* The wrong row was added to the hot list. */
                                        release_entry(cache_tree, res, 0);
                                        retain_entry(sec_entry);