refs: unregister reference stores from "chdir_notify"

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 <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/files-backend.c b/refs/files-backend.c
index a4c7858787127d1ffd76d6b898ab928a68d33afc..296981584b6f9946debfa78a0e37fdbbc2dbcb3e 100644 (file)
--- 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 0acde48c4525132b17715cc735c64a3078c27e02..499cb55dface4fd62e797bf1821c79d223f87e09 100644 (file)
--- 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 4ae22922de558b184d3627511d74c23d3ee9ce00..8c93070677dcc3ea8e13b3a4a2504fd124fc9062 100644 (file)
--- 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,