[PATCH v4 09/10] refs: fix recursing `get_main_ref_store()` with "onbranch" config
From: Patrick Steinhardt <hidden>
Date: 2026-06-19 11:28:27
Subsystem:
the rest · Maintainer:
Linus Torvalds
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 recurse:
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.
The mechanism to use the configured reference format is quite fragile in
the first place. Introduce a new mechanism that allows us to explicitly
skip evaluation of "onbranch" conditions and use it to fix the issue.
Add a sanity check in `get_main_ref_store()` to make sure we aren't
recursing, which would have failed before the fix.
Signed-off-by: Patrick Steinhardt <redacted>
---
config.c | 4 +++-
config.h | 1 +
refs.c | 7 +++++++
refs/files-backend.c | 8 +++++++-
refs/reftable-backend.c | 8 +++++++-
5 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index a1b92fe083..223c252236 100644
--- a/config.c
+++ b/config.c@@ -302,7 +302,9 @@ static int include_by_branch(struct config_include_data *data, struct strbuf pattern = STRBUF_INIT; const char *refname, *shortname; - if (!data->repo || data->repo->ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN) + if (!data->repo || + data->opts->ignore_refs || + data->repo->ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN) return 0; refname = refs_resolve_ref_unsafe(get_main_ref_store(data->repo),
diff --git a/config.h b/config.h
index bf47fb3afc..42aedde878 100644
--- a/config.h
+++ b/config.h@@ -88,6 +88,7 @@ typedef int (*config_parser_event_fn_t)(enum config_event_t type, struct config_options { unsigned int respect_includes : 1; unsigned int ignore_repo : 1; + unsigned int ignore_refs : 1; unsigned int ignore_worktree : 1; unsigned int ignore_cmdline : 1; unsigned int system_gently : 1;
diff --git a/refs.c b/refs.c
index 5b773b1c15..f242e6ca96 100644
--- a/refs.c
+++ b/refs.c@@ -2359,15 +2359,22 @@ void ref_store_release(struct ref_store *ref_store) struct ref_store *get_main_ref_store(struct repository *r) { + static bool initializing; + if (r->refs_private) return r->refs_private; if (!r->gitdir) BUG("attempting to get main_ref_store outside of repository"); + if (initializing) + BUG("main reference store creation is recursing"); + initializing = true; r->refs_private = ref_store_init(r, r->ref_storage_format, r->gitdir, REF_STORE_ALL_CAPS); r->refs_private = maybe_debug_wrap_ref_store(r->gitdir, r->refs_private); + initializing = false; + return r->refs_private; }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 79fb6735e1..ce29875cdd 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c@@ -141,6 +141,12 @@ static struct ref_store *files_ref_store_init(struct repository *repo, const char *gitdir, const struct ref_store_init_options *opts) { + struct config_options config_opts = { + .respect_includes = 1, + .ignore_refs = 1, + .commondir = repo->commondir, + .git_dir = repo->gitdir, + }; struct files_ref_store *refs = xcalloc(1, sizeof(*refs)); struct ref_store *ref_store = (struct ref_store *)refs; struct strbuf ref_common_dir = STRBUF_INIT;
@@ -158,7 +164,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo, refs->store_flags = opts->access_flags; refs->log_all_ref_updates = LOG_REFS_UNSET; - repo_config(repo, files_ref_store_config, refs); + config_with_options(files_ref_store_config, refs, NULL, repo, &config_opts); chdir_notify_register(NULL, files_ref_store_reparent, refs); strbuf_release(&refdir);
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index ee92bd9c70..05d4edc6fd 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c@@ -390,6 +390,12 @@ static struct ref_store *reftable_be_init(struct repository *repo, const char *gitdir, const struct ref_store_init_options *opts) { + struct config_options config_opts = { + .respect_includes = 1, + .ignore_refs = 1, + .commondir = repo->commondir, + .git_dir = repo->gitdir, + }; struct reftable_ref_store *refs = xcalloc(1, sizeof(*refs)); struct strbuf ref_common_dir = STRBUF_INIT; struct strbuf refdir = STRBUF_INIT;
@@ -424,7 +430,7 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->write_options.lock_timeout_ms = 100; refs->log_all_ref_updates = LOG_REFS_UNSET; - repo_config(repo, reftable_be_config, refs); + config_with_options(reftable_be_config, refs, NULL, repo, &config_opts); /* * It is somewhat unfortunate that we have to mirror the default block
--
2.55.0.rc1.722.g2b3ac350e6.dirty