From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 04 Mar 2026 07:42:43 GMT
Subject: Re: [PATCH v7 02/10] fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon
Message-ID: <aaficy025yUqsgN5@pks.im>
In-Reply-To: <d0bd3e32ca32b4150054ea91aa774a5b2db427e5.1772065643.git.gitgitgadget@gmail.com>

On Thu, Feb 26, 2026 at 12:27:15AM +0000, Paul Tarjan via GitGitGadget wrote:
> From: Paul Tarjan <github@paulisageek.com>
> 
> The `state.cookies` hashmap is initialized during daemon startup but
> never freed during cleanup in the `done:` label of
> fsmonitor_run_daemon().  Add a hashmap_clear() call to prevent this
> memory leak.
> 
> Signed-off-by: Paul Tarjan <github@paulisageek.com>
> ---
>  builtin/fsmonitor--daemon.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
> index bc4571938c..4d52622e24 100644
> --- a/builtin/fsmonitor--daemon.c
> +++ b/builtin/fsmonitor--daemon.c
> @@ -1404,6 +1404,7 @@ static int fsmonitor_run_daemon(void)
>  done:
>  	pthread_cond_destroy(&state.cookies_cond);
>  	pthread_mutex_destroy(&state.main_lock);
> +	hashmap_clear(&state.cookies);
>  	fsm_listen__dtor(&state);
>  	fsm_health__dtor(&state);

Is this actually sufficient? as far as I can see, the cookies are
inserted in `__wait_for_cookie()`, and each cookie also has a name
attached to it that was allocated via a strbuf. So don't we have to
free the name, as well?

Patrick