{"thread":{"id":"61900","subject":"[PATCH] repository: prevent memory leak when releasing ref stores","startedAt":"2024-08-05T10:56:07Z","lastAt":"2024-08-06T15:44:30Z","messageCount":7,"participants":["Sven Strickroth via GitGitGadget","Sven Strickroth","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"500056","messageId":"pull.1758.git.git.1722855364436.gitgitgadget@gmail.com","threadId":"61900","inReplyTo":null,"subject":"[PATCH] repository: prevent memory leak when releasing ref stores","fromName":"Sven Strickroth via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-08-05T10:56:04Z","receivedAt":"2024-08-05T10:56:07Z","isPatch":true,"sender":{"key":"sven.strickroth@tu-clausthal.de","avatar":null},"body":"From: Sven Strickroth <email@cs-ware.de>\n\n`ref_store_release` does not free the ref_store allocated in\n`ref_store_init`.\n\nSigned-off-by: Sven Strickroth <email@cs-ware.de>\n---\n    repository: prevent memory leak when releasing ref stores\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1758%2Fcsware%2Frepository-memory-leak-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1758/csware/repository-memory-leak-v1\nPull-Request: https://github.com/git/git/pull/1758\n\n repository.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/repository.c b/repository.c\nindex 9825a308993..46f1eadfe95 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -366,12 +366,16 @@ void repo_clear(struct repository *repo)\n \t\tFREE_AND_NULL(repo->remote_state);\n \t}\n \n-\tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)\n+\tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e) {\n \t\tref_store_release(e->value);\n+\t\tfree(e->value);\n+\t}\n \tstrmap_clear(&repo->submodule_ref_stores, 1);\n \n-\tstrmap_for_each_entry(&repo->worktree_ref_stores, &iter, e)\n+\tstrmap_for_each_entry(&repo->worktree_ref_stores, &iter, e) {\n \t\tref_store_release(e->value);\n+\t\tfree(e->value);\n+\t}\n \tstrmap_clear(&repo->worktree_ref_stores, 1);\n \n \trepo_clear_path_cache(&repo->cached_paths);\n\nbase-commit: e559c4bf1a306cf5814418d318cc0fea070da3c7\n-- \ngitgitgadget\n"},{"id":"500091","messageId":"8594c7bb-07ed-4c54-8712-5b0d4299b8eb@cs-ware.de","threadId":"61900","inReplyTo":"pull.1758.git.git.1722855364436.gitgitgadget@gmail.com","subject":"Re: [PATCH] repository: prevent memory leak when releasing ref stores","fromName":"Sven Strickroth","fromEmail":"email@cs-ware.de","sentAt":"2024-08-05T15:50:04Z","receivedAt":"2024-08-05T15:50:07Z","isPatch":true,"sender":{"key":"email@cs-ware.de","avatar":"https://avatars.githubusercontent.com/u/428133?v=4"},"body":"Am 05.08.2024 um 12:56 schrieb Sven Strickroth via GitGitGadget:\n> -\tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)\n> +\tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e) {\n>   \t\tref_store_release(e->value);\n> +\t\tfree(e->value);\n> +\t}\n>   \tstrmap_clear(&repo->submodule_ref_stores, 1);\n\nAfter further checking this does not seem to be necessary. The ref \nstores are already free'd in strmap_clear.\n\n-- \nBest regards,\n  Sven Strickroth\n  PGP key id F5A9D4C4 @ any key-server\n"},{"id":"500097","messageId":"xmqq34nj3pez.fsf@gitster.g","threadId":"61900","inReplyTo":"pull.1758.git.git.1722855364436.gitgitgadget@gmail.com","subject":"Re: [PATCH] repository: prevent memory leak when releasing ref stores","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-05T16:28:04Z","receivedAt":"2024-08-05T16:28:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sven Strickroth via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Sven Strickroth <email@cs-ware.de>\n>\n> `ref_store_release` does not free the ref_store allocated in\n> `ref_store_init`.\n>\n> Signed-off-by: Sven Strickroth <email@cs-ware.de>\n> ---\n\nThis may certainly plug the two leaking callers, but stepping back a\nbit and looking at other existing calls to ref_store_release(), I\nwonder if many existing and more importantly future callers benefit\nif ref_store_release() did the freeing of the surrounding shell, as\nwe can see in these existing calls:\n\nrefs.c:2851:\tref_store_release(new_refs);\nrefs.c-2852-\tFREE_AND_NULL(new_refs);\n\nrefs.c:2890:\tref_store_release(old_refs);\nrefs.c-2891-\tFREE_AND_NULL(old_refs);\n\nrefs.c:2904:\t\tref_store_release(new_refs);\nrefs.c-2905-\t\tfree(new_refs);\n\nIf we change the type of ref_store_release() to take a pointer to a\npointer to ref_store, so that the above callers can just become\n\n\tref_store_release(&new_refs);\n\nto release the resources and new_refs variable cleared, the\ncallsites in this patch can do the same.\n\nHowever, I am fuzzy on the existing uses in the backend\nimplementation.  For example:\n\n        static void files_ref_store_release(struct ref_store *ref_store)\n        {\n                struct files_ref_store *refs = files_downcast(ref_store, 0, \"release\");\n                free_ref_cache(refs->loose);\n                free(refs->gitcommondir);\n                ref_store_release(refs->packed_ref_store);\n        }\n\nThe packed-ref-store is \"released\" here, as part of \"releasing\" the\nfiles-ref-store that uses it as a fallback backend.  The caller of\nfiles_ref_store_release() is refs.c:ref_store_release()\n\n        void ref_store_release(struct ref_store *ref_store)\n        {\n                ref_store->be->release(ref_store);\n                free(ref_store->gitdir);\n        }\n\nSo if you have a files based ref store, when you are done you'd be\ncalling ref_store_release() on it, releasing the resources held by\nthe files_ref_store structure, but I do not know who frees the\npacked_ref_store allocated by files_ref_store_init().  Perhaps it is\nalready leaking?  If that is the case then an API update like I\nsuggested above would make even more sense to make it less likely\nfor such a leak to be added to the system in the future, I suspect.\n\nI dunno.\n\nThanks.\n\n>     repository: prevent memory leak when releasing ref stores\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1758%2Fcsware%2Frepository-memory-leak-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1758/csware/repository-memory-leak-v1\n> Pull-Request: https://github.com/git/git/pull/1758\n>\n>  repository.c | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/repository.c b/repository.c\n> index 9825a308993..46f1eadfe95 100644\n> --- a/repository.c\n> +++ b/repository.c\n> @@ -366,12 +366,16 @@ void repo_clear(struct repository *repo)\n>  \t\tFREE_AND_NULL(repo->remote_state);\n>  \t}\n>  \n> -\tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)\n> +\tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e) {\n>  \t\tref_store_release(e->value);\n> +\t\tfree(e->value);\n> +\t}\n>  \tstrmap_clear(&repo->submodule_ref_stores, 1);\n>  \n> -\tstrmap_for_each_entry(&repo->worktree_ref_stores, &iter, e)\n> +\tstrmap_for_each_entry(&repo->worktree_ref_stores, &iter, e) {\n>  \t\tref_store_release(e->value);\n> +\t\tfree(e->value);\n> +\t}\n>  \tstrmap_clear(&repo->worktree_ref_stores, 1);\n>  \n>  \trepo_clear_path_cache(&repo->cached_paths);\n>\n> base-commit: e559c4bf1a306cf5814418d318cc0fea070da3c7\n"},{"id":"500114","messageId":"xmqqed723mth.fsf@gitster.g","threadId":"61900","inReplyTo":"xmqq34nj3pez.fsf@gitster.g","subject":"Re: [PATCH] repository: prevent memory leak when releasing ref stores","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-05T17:24:10Z","receivedAt":"2024-08-05T17:24:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> However, I am fuzzy on the existing uses in the backend\n> implementation.  For example:\n>\n>         static void files_ref_store_release(struct ref_store *ref_store)\n>         {\n>                 struct files_ref_store *refs = files_downcast(ref_store, 0, \"release\");\n>                 free_ref_cache(refs->loose);\n>                 free(refs->gitcommondir);\n>                 ref_store_release(refs->packed_ref_store);\n>         }\n>\n> The packed-ref-store is \"released\" here, as part of \"releasing\" the\n> files-ref-store that uses it as a fallback backend.  The caller of\n> files_ref_store_release() is refs.c:ref_store_release()\n>\n>         void ref_store_release(struct ref_store *ref_store)\n>         {\n>                 ref_store->be->release(ref_store);\n>                 free(ref_store->gitdir);\n>         }\n>\n> So if you have a files based ref store, when you are done you'd be\n> calling ref_store_release() on it, releasing the resources held by\n> the files_ref_store structure, but I do not know who frees the\n> packed_ref_store allocated by files_ref_store_init().  Perhaps it is\n> already leaking?  If that is the case then an API update like I\n> suggested above would make even more sense to make it less likely\n> for such a leak to be added to the system in the future, I suspect.\n\nAhh, that was the leak that you plugged in a separate patch.\n\nSo it does point us in the other direction to redefine _release with\na different behaviour that releases the resource held by the\nstructure, and frees the structure itself.\n\nSomething along the following line (caution: totally untested) that\nallows your two patches to become empty, and also allows a few\ncallers to lose their existing explicit free()s immediately after\nthey call _release(), perhaps?\n\nIf this were to become a real patch, I think debug backend should\nlearn to use the same _downcast() to become more like the real ones\nbefore it happens in a preliminary clean-up patch.\n\n refs.h                  |  5 +++--\n refs.c                  | 19 ++++++++-----------\n refs/refs-internal.h    |  2 +-\n refs/files-backend.c    |  6 +++---\n refs/packed-backend.c   |  5 +++--\n refs/reftable-backend.c |  6 +++---\n refs/debug.c            |  6 +++---\n 7 files changed, 24 insertions(+), 25 deletions(-)\n\ndiff --git c/refs.h w/refs.h\nindex b3e39bc257..e4f092f6ac 100644\n--- c/refs.h\n+++ w/refs.h\n@@ -119,9 +119,10 @@ int is_branch(const char *refname);\n int ref_store_create_on_disk(struct ref_store *refs, int flags, struct strbuf *err);\n \n /*\n- * Release all memory and resources associated with the ref store.\n+ * Release all memory and resources associated with the ref store, including\n+ * the ref_store itself.\n  */\n-void ref_store_release(struct ref_store *ref_store);\n+void ref_store_release(struct ref_store **ref_store);\n \n /*\n  * Remove the ref store from disk. This deletes all associated data.\ndiff --git c/refs.c w/refs.c\nindex 915aeb4d1d..cb76a5d4bd 100644\n--- c/refs.c\n+++ w/refs.c\n@@ -1936,10 +1936,11 @@ static struct ref_store *ref_store_init(struct repository *repo,\n \treturn refs;\n }\n \n-void ref_store_release(struct ref_store *ref_store)\n+void ref_store_release(struct ref_store **ref_store)\n {\n-\tref_store->be->release(ref_store);\n-\tfree(ref_store->gitdir);\n+\t(*ref_store)->be->release(ref_store);\n+\tfree((*ref_store)->gitdir);\n+\tFREE_AND_NULL(*ref_store);\n }\n \n struct ref_store *get_main_ref_store(struct repository *r)\n@@ -2848,8 +2849,7 @@ int repo_migrate_ref_storage_format(struct repository *repo,\n \t * be closed. This is required for platforms like Cygwin, where\n \t * renaming an open file results in EPERM.\n \t */\n-\tref_store_release(new_refs);\n-\tFREE_AND_NULL(new_refs);\n+\tref_store_release(&new_refs);\n \n \t/*\n \t * Until now we were in the non-destructive phase, where we only\n@@ -2887,8 +2887,7 @@ int repo_migrate_ref_storage_format(struct repository *repo,\n \t * make sure to lazily re-initialize the repository's ref store with\n \t * the new format.\n \t */\n-\tref_store_release(old_refs);\n-\tFREE_AND_NULL(old_refs);\n+\tref_store_release(&old_refs);\n \trepo->refs_private = NULL;\n \n \tret = 0;\n@@ -2900,10 +2899,8 @@ int repo_migrate_ref_storage_format(struct repository *repo,\n \t\t\t    new_gitdir.buf);\n \t}\n \n-\tif (new_refs) {\n-\t\tref_store_release(new_refs);\n-\t\tfree(new_refs);\n-\t}\n+\tif (new_refs)\n+\t\tref_store_release(&new_refs);\n \tref_transaction_free(transaction);\n \tstrbuf_release(&new_gitdir);\n \treturn ret;\ndiff --git c/refs/refs-internal.h w/refs/refs-internal.h\nindex fa975d69aa..2ba2372acb 100644\n--- c/refs/refs-internal.h\n+++ w/refs/refs-internal.h\n@@ -511,7 +511,7 @@ typedef struct ref_store *ref_store_init_fn(struct repository *repo,\n /*\n  * Release all memory and resources associated with the ref store.\n  */\n-typedef void ref_store_release_fn(struct ref_store *refs);\n+typedef void ref_store_release_fn(struct ref_store **refs);\n \n typedef int ref_store_create_on_disk_fn(struct ref_store *refs,\n \t\t\t\t\tint flags,\ndiff --git c/refs/files-backend.c w/refs/files-backend.c\nindex aa52d9be7c..8ebb1681ac 100644\n--- c/refs/files-backend.c\n+++ w/refs/files-backend.c\n@@ -151,12 +151,12 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,\n \treturn refs;\n }\n \n-static void files_ref_store_release(struct ref_store *ref_store)\n+static void files_ref_store_release(struct ref_store **ref_store)\n {\n-\tstruct files_ref_store *refs = files_downcast(ref_store, 0, \"release\");\n+\tstruct files_ref_store *refs = files_downcast(*ref_store, 0, \"release\");\n \tfree_ref_cache(refs->loose);\n \tfree(refs->gitcommondir);\n-\tref_store_release(refs->packed_ref_store);\n+\tref_store_release(&refs->packed_ref_store);\n }\n \n static void files_reflog_path(struct files_ref_store *refs,\ndiff --git c/refs/packed-backend.c w/refs/packed-backend.c\nindex a0666407cd..8321a4cc17 100644\n--- c/refs/packed-backend.c\n+++ w/refs/packed-backend.c\n@@ -260,13 +260,14 @@ static void clear_snapshot(struct packed_ref_store *refs)\n \t}\n }\n \n-static void packed_ref_store_release(struct ref_store *ref_store)\n+static void packed_ref_store_release(struct ref_store **ref_store)\n {\n-\tstruct packed_ref_store *refs = packed_downcast(ref_store, 0, \"release\");\n+\tstruct packed_ref_store *refs = packed_downcast(*ref_store, 0, \"release\");\n \tclear_snapshot(refs);\n \trollback_lock_file(&refs->lock);\n \tdelete_tempfile(&refs->tempfile);\n \tfree(refs->path);\n+\tFREE_AND_NULL(*ref_store);\n }\n \n static NORETURN void die_unterminated_line(const char *path,\ndiff --git c/refs/reftable-backend.c w/refs/reftable-backend.c\nindex fbe74c239d..0af010acfb 100644\n--- c/refs/reftable-backend.c\n+++ w/refs/reftable-backend.c\n@@ -337,9 +337,9 @@ static struct ref_store *reftable_be_init(struct repository *repo,\n \treturn &refs->base;\n }\n \n-static void reftable_be_release(struct ref_store *ref_store)\n+static void reftable_be_release(struct ref_store **ref_store)\n {\n-\tstruct reftable_ref_store *refs = reftable_be_downcast(ref_store, 0, \"release\");\n+\tstruct reftable_ref_store *refs = reftable_be_downcast(*ref_store, 0, \"release\");\n \tstruct strmap_entry *entry;\n \tstruct hashmap_iter iter;\n \n@@ -400,7 +400,7 @@ static int reftable_be_remove_on_disk(struct ref_store *ref_store,\n \t * required so that the \"tables.list\" file is not open anymore, which\n \t * would otherwise make it impossible to remove the file on Windows.\n \t */\n-\treftable_be_release(ref_store);\n+\treftable_be_release(&ref_store);\n \n \tstrbuf_addf(&sb, \"%s/reftable\", refs->base.gitdir);\n \tif (remove_dir_recursively(&sb, 0) < 0) {\ndiff --git c/refs/debug.c w/refs/debug.c\nindex 547d9245b9..be6230045c 100644\n--- c/refs/debug.c\n+++ w/refs/debug.c\n@@ -33,10 +33,10 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor\n \treturn (struct ref_store *)res;\n }\n \n-static void debug_release(struct ref_store *refs)\n+static void debug_release(struct ref_store **refs)\n {\n-\tstruct debug_ref_store *drefs = (struct debug_ref_store *)refs;\n-\tdrefs->refs->be->release(drefs->refs);\n+\tstruct debug_ref_store *drefs = *(struct debug_ref_store **)refs;\n+\tdrefs->refs->be->release(&drefs->refs);\n \ttrace_printf_key(&trace_refs, \"release\\n\");\n }\n \n"},{"id":"500115","messageId":"xmqqa5hq3lzq.fsf@gitster.g","threadId":"61900","inReplyTo":"8594c7bb-07ed-4c54-8712-5b0d4299b8eb@cs-ware.de","subject":"Re: [PATCH] repository: prevent memory leak when releasing ref stores","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-05T17:42:01Z","receivedAt":"2024-08-05T17:42:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth <email@cs-ware.de> writes:\n\n> Am 05.08.2024 um 12:56 schrieb Sven Strickroth via GitGitGadget:\n>> -\tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)\n>> +\tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e) {\n>>   \t\tref_store_release(e->value);\n>> +\t\tfree(e->value);\n>> +\t}\n>>   \tstrmap_clear(&repo->submodule_ref_stores, 1);\n>\n> After further checking this does not seem to be necessary. The ref\n> stores are already free'd in strmap_clear.\n\nIs it \"not necessary\" or \"actively harmful\"?  It sounds like the\nlatter?\n"},{"id":"500155","messageId":"ZrHAu0wfipR6CShS@tanuki","threadId":"61900","inReplyTo":"xmqqed723mth.fsf@gitster.g","subject":"Re: [PATCH] repository: prevent memory leak when releasing ref stores","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-06T06:20:43Z","receivedAt":"2024-08-06T06:20:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Aug 05, 2024 at 10:24:10AM -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > However, I am fuzzy on the existing uses in the backend\n> > implementation.  For example:\n> >\n> >         static void files_ref_store_release(struct ref_store *ref_store)\n> >         {\n> >                 struct files_ref_store *refs = files_downcast(ref_store, 0, \"release\");\n> >                 free_ref_cache(refs->loose);\n> >                 free(refs->gitcommondir);\n> >                 ref_store_release(refs->packed_ref_store);\n> >         }\n> >\n> > The packed-ref-store is \"released\" here, as part of \"releasing\" the\n> > files-ref-store that uses it as a fallback backend.  The caller of\n> > files_ref_store_release() is refs.c:ref_store_release()\n> >\n> >         void ref_store_release(struct ref_store *ref_store)\n> >         {\n> >                 ref_store->be->release(ref_store);\n> >                 free(ref_store->gitdir);\n> >         }\n> >\n> > So if you have a files based ref store, when you are done you'd be\n> > calling ref_store_release() on it, releasing the resources held by\n> > the files_ref_store structure, but I do not know who frees the\n> > packed_ref_store allocated by files_ref_store_init().  Perhaps it is\n> > already leaking?  If that is the case then an API update like I\n> > suggested above would make even more sense to make it less likely\n> > for such a leak to be added to the system in the future, I suspect.\n> \n> Ahh, that was the leak that you plugged in a separate patch.\n> \n> So it does point us in the other direction to redefine _release with\n> a different behaviour that releases the resource held by the\n> structure, and frees the structure itself.\n> \n> Something along the following line (caution: totally untested) that\n> allows your two patches to become empty, and also allows a few\n> callers to lose their existing explicit free()s immediately after\n> they call _release(), perhaps?\n\nI don't really know whether it's worth the churn, but if somebody wants\nto pull through with this I'm game :) But: if we are going to do this,\nwe should rename the function to be called `ref_store_free()` instead of\n`ref_store_release()` according to our recent coding style update :)\n\n> If this were to become a real patch, I think debug backend should\n> learn to use the same _downcast() to become more like the real ones\n> before it happens in a preliminary clean-up patch.\n\nThat certainly wouldn't hurt, yeah.\n\nPatrick\n"},{"id":"500230","messageId":"xmqqcymlzmec.fsf@gitster.g","threadId":"61900","inReplyTo":"ZrHAu0wfipR6CShS@tanuki","subject":"Re: [PATCH] repository: prevent memory leak when releasing ref stores","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-06T15:44:27Z","receivedAt":"2024-08-06T15:44:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> Something along the following line (caution: totally untested) that\n>> allows your two patches to become empty, and also allows a few\n>> callers to lose their existing explicit free()s immediately after\n>> they call _release(), perhaps?\n>\n> I don't really know whether it's worth the churn, but if somebody wants\n> to pull through with this I'm game :) But: if we are going to do this,\n> we should rename the function to be called `ref_store_free()` instead of\n> `ref_store_release()` according to our recent coding style update :)\n\nYes, we had \"what is release, clear, and free?\" discussion recently.\n\n>> If this were to become a real patch, I think debug backend should\n>> learn to use the same _downcast() to become more like the real ones\n>> before it happens in a preliminary clean-up patch.\n>\n> That certainly wouldn't hurt, yeah.\n\nI am not short of (other) things to do, and expect that I will not\ntouching this for a while, but in case somebody finds #leftoverbits\nhere, I'll leave a note here.  \n\nThere are \"hidden\" freeing that we have to adjust, if we were to\nfollow through this approach.  For example, those free()'s added in\nthe patch in the message that started the thread are introduing\ndouble free---after the strmap_for_each_entry() loop, a\nstrmap_clear() callis done with free_values=1.  If we freed inside\nref_store_release(), we'd need to adjust the call to strmap_clear()\nto pass free_values=0 to compensate.\n"}]}