From 238191ecfc104f5647bf0dc8c1d7d8921ce03d96 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 10 Jun 2026 16:57:13 +0200 Subject: [PATCH] refs: fix recursing `get_main_ref_store()` with "onbranch" config When we have an "onbranch" condition we need to ask the reference database whether HEAD currently points at the configured branch. This unfortunately creates a chicken-and-egg problem: - The reference database needs to read the configuration so that it can configure itself. - The configuration needs to construct a reference database to fully parse all of its conditionals. The way we handle this is by simply excluding "onbranch" conditionals when we haven't yet configured the reference database. The mechanism for this is broken though: to verify whether or not we have configured the reference database we check whether its format is set to `REF_STORAGE_UNKNOWN` in `include_by_branch()`. But typically, the format _is_ already known at that time because we set it up during repository discovery in "setup.c". The consequence is that we have recursion: 1. We call `get_main_ref_store()`. 2. We don't yet have a reference store, so we call `ref_store_init()`. 3. We parse the configuration required for the reference store. 4. We eventually end up in `include_by_branch()`. 5. We have already configured the reference storage format, so we end up calling `get_main_ref_store()` again. We still haven't finished (1) though, so `get_main_ref_store()` will now call `ref_store_init()` a second time. The end result is that we have constructed the same reference store twice. Of course, as both reference stores would be assigned to `refs_private`, we leak one of those two instances. This never surfaced as an actual leak though because the pointer is kept alive by the "chdir_notify" subsystem. For now, we can fix the issue by explicitly unsetting the reference storage format before constructing it. This makes the mentioned check trigger as expected, and consequently we won't end up constructing a second reference database at all. Ultimately, this means that we consistently stop evaluating "onbranch" conditions when constructing the main reference database. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index d3caa9a633..e69b9b8ac8 100644 --- a/refs.c +++ b/refs.c @@ -2351,15 +2351,31 @@ void ref_store_release(struct ref_store *ref_store) struct ref_store *get_main_ref_store(struct repository *r) { + enum ref_storage_format format; + if (r->refs_private) return r->refs_private; if (!r->gitdir) BUG("attempting to get main_ref_store outside of repository"); - r->refs_private = ref_store_init(r, r->ref_storage_format, - r->gitdir, REF_STORE_ALL_CAPS); + /* + * When constructing the reference backend we'll end up reading the Git + * configuration. This means we'll also try to evaluate "onbranch" + * conditions. + * + * We cannot read branches when constructing the refdb, so it is not + * possible to evaluate those conditions in the first place. To gate + * their evaluation we check whether or not the reference storage + * format has been configured -- we thus have to temporarily set it to + * UNKNOWN here so that we don't end up recursing. + */ + format = r->ref_storage_format; + r->ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN; + r->refs_private = ref_store_init(r, format, r->gitdir, REF_STORE_ALL_CAPS); r->refs_private = maybe_debug_wrap_ref_store(r->gitdir, r->refs_private); + r->ref_storage_format = format; + return r->refs_private; }