Re: [PATCH 03/16] refs: implement releasing ref storages
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- May 16, 2024, 16:39 UTC
- Message-ID
- <CAOLa=ZT3VqhrHXt9x-4DN7norTb3RuscBx+VHipn=QxJvwwThQ@mail.gmail.com>
- In-Reply-To
- <282fbe35a7c9db715a8a805f93f9b465d42885a5.1715836916.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
[snip]
Show 11 quoted lines
> diff --git a/refs/debug.c b/refs/debug.c
> index 58fb4557ed..27aae42134 100644
> --- a/refs/debug.c
> +++ b/refs/debug.c
> @@ -33,6 +33,12 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor
> return (struct ref_store *)res;
> }
>
> +static void debug_release(struct ref_store *refs)
> +{
> + struct debug_ref_store *drefs = (struct debug_ref_store *)refs;We should probably add a trace here, using `trace_printf_key()`
Show 57 quoted lines
> + drefs->refs->be->release(drefs->refs);
> +}
> +
> static int debug_create(struct ref_store *refs, int flags, struct strbuf *err)
> {
> struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
> @@ -427,6 +433,7 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname,
> struct ref_storage_be refs_be_debug = {
> .name = "debug",
> .init = NULL,
> + .release = debug_release,
> .create = debug_create,
>
> /*
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index f766d18d5a..368df075c1 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -149,6 +149,14 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,
> return refs;
> }
>
> +static void files_ref_store_release(struct ref_store *ref_store)
> +{
> + struct files_ref_store *refs = files_downcast(ref_store, 0, "release");
> + free_ref_cache(refs->loose);
> + free(refs->gitcommondir);
> + ref_store_release(refs->packed_ref_store);
> +}
> +
> static void files_reflog_path(struct files_ref_store *refs,
> struct strbuf *sb,
> const char *refname)
> @@ -3284,6 +3292,7 @@ static int files_ref_store_create(struct ref_store *ref_store,
> struct ref_storage_be refs_be_files = {
> .name = "files",
> .init = files_ref_store_init,
> + .release = files_ref_store_release,
> .create = files_ref_store_create,
> .transaction_prepare = files_transaction_prepare,
> .transaction_finish = files_transaction_finish,
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index 716513efed..bebceb4aa7 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -252,6 +252,15 @@ static void clear_snapshot(struct packed_ref_store *refs)
> }
> }
>
> +static void packed_ref_store_release(struct ref_store *ref_store)
> +{
> + struct packed_ref_store *refs = packed_downcast(ref_store, 0, "release");
> + clear_snapshot(refs);
> + rollback_lock_file(&refs->lock);
> + delete_tempfile(&refs->tempfile);
> + free(refs->path);
> +}Verified that all the params inside `packed_ref_store` are cleaned up here, nice!
Show 57 quoted lines
> static NORETURN void die_unterminated_line(const char *path,
> const char *p, size_t len)
> {
> @@ -1707,6 +1716,7 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s
> struct ref_storage_be refs_be_packed = {
> .name = "packed",
> .init = packed_ref_store_init,
> + .release = packed_ref_store_release,
> .create = packed_ref_store_create,
> .transaction_prepare = packed_transaction_prepare,
> .transaction_finish = packed_transaction_finish,
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index eb42212764..cc1fe6e633 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -530,6 +530,11 @@ typedef struct ref_store *ref_store_init_fn(struct repository *repo,
> const char *gitdir,
> unsigned int flags);
>
> +/*
> + * Release all memory and resources associated with the ref store.
> + */
> +typedef void ref_store_release_fn(struct ref_store *refs);
> +
> typedef int ref_store_create_fn(struct ref_store *refs,
> int flags,
> struct strbuf *err);
> @@ -668,6 +673,7 @@ typedef int read_symbolic_ref_fn(struct ref_store *ref_store, const char *refnam
> struct ref_storage_be {
> const char *name;
> ref_store_init_fn *init;
> + ref_store_release_fn *release;
> ref_store_create_fn *create;
>
> ref_transaction_prepare_fn *transaction_prepare;
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index a4bb71cd76..6c262c2193 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -293,6 +293,27 @@ static struct ref_store *reftable_be_init(struct repository *repo,
> return &refs->base;
> }
>
> +static void reftable_be_release(struct ref_store *ref_store)
> +{
> + struct reftable_ref_store *refs = reftable_be_downcast(ref_store, 0, "release");
> + struct strmap_entry *entry;
> + struct hashmap_iter iter;
> +
> + if (refs->main_stack) {
> + reftable_stack_destroy(refs->main_stack);
> + refs->main_stack = NULL;
> + }
> +
> + if (refs->worktree_stack) {
> + reftable_stack_destroy(refs->worktree_stack);
> + refs->main_stack = NULL;This should be `refs->worktree_stack`, right?
Show 20 quoted lines
> + }
> +
> + strmap_for_each_entry(&refs->worktree_stacks, &iter, entry)
> + reftable_stack_destroy(entry->value);
> + strmap_clear(&refs->worktree_stacks, 0);
> +}
> +
> static int reftable_be_create(struct ref_store *ref_store,
> int flags UNUSED,
> struct strbuf *err UNUSED)
> @@ -2248,6 +2269,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
> struct ref_storage_be refs_be_reftable = {
> .name = "reftable",
> .init = reftable_be_init,
> + .release = reftable_be_release,
> .create = reftable_be_create,
> .transaction_prepare = reftable_be_transaction_prepare,
> .transaction_finish = reftable_be_transaction_finish,
> --
> 2.45.1.190.g19fe900cfc.dirty