[PATCH v3 4/8] refs: unregister reference stores from "chdir_notify"
From: Patrick Steinhardt <hidden>
Date: 2026-06-18 06:54:49
Subsystem:
the rest · Maintainer:
Linus Torvalds
When creating reference stores we register them with the "chdir_notify"
subsystem. This is required because some of the paths we track may be
relative paths, so we have to reparent them in case the current working
directory changes.
But while we register the reference stores, we never unregister them.
This can have multiple outcomes:
- For a repository's main reference database we essentially keep the
pointer alive. We never free that database, either, and our leak
checker doesn't notice because it's still registered.
- For submodule and worktree reference databases we do eventually free
them in `repo_clear()`, so we may keep pointers to free'd memory
registered. We never notice though as we don't tend to chdir around
in the middle of the process.
We never noticed either of these symptoms, but they are obviously bad.
Partially fix those issues by unregistering the reference stores when
releasing them. The leak of the main reference database will be fixed in
a subsequent commit.
Note that this requires us to use `chdir_notify_register()` instead of
`chdir_notify_reparent()`, as there is no infrastructure to unregister the
latter.
Signed-off-by: Patrick Steinhardt <redacted>
---
refs/files-backend.c | 22 +++++++++++++++++++---
refs/packed-backend.c | 16 +++++++++++++++-
refs/reftable-backend.c | 16 +++++++++++++++-
3 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a4c7858787..296981584b 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c@@ -100,6 +100,23 @@ static void clear_loose_ref_cache(struct files_ref_store *refs) } } +static void files_ref_store_reparent(const char *name UNUSED, + const char *old_cwd, + const char *new_cwd, + void *payload) +{ + struct files_ref_store *refs = payload; + char *tmp; + + tmp = reparent_relative_path(old_cwd, new_cwd, refs->base.gitdir); + free(refs->base.gitdir); + refs->base.gitdir = tmp; + + tmp = reparent_relative_path(old_cwd, new_cwd, refs->gitcommondir); + free(refs->gitcommondir); + refs->gitcommondir = tmp; +} + /* * Create a new submodule ref cache and add it to the internal * set of caches.
@@ -128,9 +145,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo, repo_config_get_bool(repo, "core.prefersymlinkrefs", &refs->prefer_symlink_refs); - chdir_notify_reparent("files-backend $GIT_DIR", &refs->base.gitdir); - chdir_notify_reparent("files-backend $GIT_COMMONDIR", - &refs->gitcommondir); + chdir_notify_register(NULL, files_ref_store_reparent, refs); strbuf_release(&refdir);
@@ -182,6 +197,7 @@ static void files_ref_store_release(struct ref_store *ref_store) free(refs->gitcommondir); ref_store_release(refs->packed_ref_store); free(refs->packed_ref_store); + chdir_notify_unregister(NULL, files_ref_store_reparent, refs); } static void files_reflog_path(struct files_ref_store *refs,
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 0acde48c45..499cb55dfa 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c@@ -211,6 +211,19 @@ static size_t snapshot_hexsz(const struct snapshot *snapshot) return snapshot->refs->base.repo->hash_algo->hexsz; } +static void packed_ref_store_reparent(const char *name UNUSED, + const char *old_cwd, + const char *new_cwd, + void *payload) +{ + struct packed_ref_store *refs = payload; + char *tmp; + + tmp = reparent_relative_path(old_cwd, new_cwd, refs->path); + free(refs->path); + refs->path = tmp; +} + /* * Since packed-refs is only stored in the common dir, don't parse the * payload and rely on the files-backend to set 'gitdir' correctly.
@@ -229,7 +242,7 @@ struct ref_store *packed_ref_store_init(struct repository *repo, strbuf_addf(&sb, "%s/packed-refs", gitdir); refs->path = strbuf_detach(&sb, NULL); - chdir_notify_reparent("packed-refs", &refs->path); + chdir_notify_register(NULL, packed_ref_store_reparent, refs); return ref_store; }
@@ -274,6 +287,7 @@ static void packed_ref_store_release(struct ref_store *ref_store) clear_snapshot(refs); rollback_lock_file(&refs->lock); delete_tempfile(&refs->tempfile); + chdir_notify_unregister(NULL, packed_ref_store_reparent, refs); free(refs->path); }
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 4ae22922de..8c93070677 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c@@ -365,6 +365,19 @@ static int reftable_be_config(const char *var, const char *value, return 0; } +static void reftable_be_reparent(const char *name UNUSED, + const char *old_cwd, + const char *new_cwd, + void *payload) +{ + struct reftable_ref_store *refs = payload; + char *tmp; + + tmp = reparent_relative_path(old_cwd, new_cwd, refs->base.gitdir); + free(refs->base.gitdir); + refs->base.gitdir = tmp; +} + static struct ref_store *reftable_be_init(struct repository *repo, const char *payload, const char *gitdir,
@@ -447,7 +460,7 @@ static struct ref_store *reftable_be_init(struct repository *repo, goto done; } - chdir_notify_reparent("reftables-backend $GIT_DIR", &refs->base.gitdir); + chdir_notify_register(NULL, reftable_be_reparent, refs); done: assert(refs->err != REFTABLE_API_ERROR);
@@ -474,6 +487,7 @@ static void reftable_be_release(struct ref_store *ref_store) free(be); } strmap_clear(&refs->worktree_backends, 0); + chdir_notify_unregister(NULL, reftable_be_reparent, refs); } static int reftable_be_create_on_disk(struct ref_store *ref_store,
--
2.55.0.rc0.786.g65d90a0328.dirty