From: Paul Tarjan via GitGitGadget Date: Wed, 31 Dec 2025 14:39:03 GMT Subject: [PATCH v2] fsmonitor: fix khash memory leak in do_handle_client Message-ID: In-Reply-To: From: Paul Tarjan The do_handle_client() function allocates a khash table to de-duplicate pathnames when responding to client requests. However, kh_release_str() was used instead of kh_destroy_str(). The release function only frees internal arrays (flags, keys, vals) but not the struct itself, which is allocated by kh_init_str() via xcalloc. This caused a 40-byte leak per client request. Fix by using kh_destroy_str() which properly frees both internal arrays and the struct itself. Also move the cleanup to the cleanup section and initialize shown to NULL so that kh_destroy_str() is safe to call on all exit paths. Signed-off-by: Paul Tarjan --- fsmonitor: fix khash memory leak in do_handle_client The do_handle_client() function allocates a khash table to de-duplicate pathnames when responding to client requests. However, kh_release_str() was used instead of kh_destroy_str(). The release function only frees internal arrays (flags, keys, vals) but not the struct itself, which is allocated by kh_init_str() via xcalloc. This caused a 40-byte leak per client request. Fix by using kh_destroy_str() which properly frees both internal arrays and the struct itself. Also move the cleanup to the cleanup section and initialize shown to NULL so that kh_destroy_str() is safe to call on all exit paths. Changes since v1: * Removed incorrect claim about interrupted threads reaching cleanup * Simplified commit message to focus on the real bug (kh_release vs kh_destroy) Signed-off-by: Paul Tarjan github@paulisageek.com Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2148%2Fptarjan%2Fclaude%2Ffix-fsmonitor-hashmap-leak-gfDCU-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2148/ptarjan/claude/fix-fsmonitor-hashmap-leak-gfDCU-v2 Pull-Request: https://github.com/git/git/pull/2148 Range-diff vs v1: 1: 18670d3230 ! 1: 0a754c7f09 fsmonitor: fix khash memory leak in do_handle_client @@ Commit message fsmonitor: fix khash memory leak in do_handle_client The do_handle_client() function allocates a khash table to de-duplicate - pathnames when responding to client requests. Two issues existed: + pathnames when responding to client requests. However, kh_release_str() + was used instead of kh_destroy_str(). The release function only frees + internal arrays (flags, keys, vals) but not the struct itself, which is + allocated by kh_init_str() via xcalloc. This caused a 40-byte leak per + client request. - 1. kh_release_str() was used instead of kh_destroy_str(). The release - function only frees internal arrays (flags, keys, vals) but not the - struct itself (allocated by kh_init_str via xcalloc). This caused a - 40-byte leak per request. + Fix by using kh_destroy_str() which properly frees both internal arrays + and the struct itself. Also move the cleanup to the cleanup section and + initialize shown to NULL so that kh_destroy_str() is safe to call on all + exit paths. - 2. The khash was freed mid-function rather than in the cleanup section, - so if the worker thread was interrupted before reaching that point - during daemon shutdown, the memory would leak. - - Fix both issues by: - - Initializing shown = NULL at declaration - - Using kh_destroy_str() which handles NULL and frees both internal - arrays and the struct itself - - Moving the cleanup to the cleanup section so it runs on all exit paths - - Signed-off-by: Claude + Signed-off-by: Paul Tarjan ## builtin/fsmonitor--daemon.c ## @@ builtin/fsmonitor--daemon.c: static int do_handle_client(struct fsmonitor_daemon_state *state, builtin/fsmonitor--daemon.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 242c594646..bc4571938c 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -671,7 +671,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, const struct fsmonitor_batch *batch; struct fsmonitor_batch *remainder = NULL; intmax_t count = 0, duplicates = 0; - kh_str_t *shown; + kh_str_t *shown = NULL; int hash_ret; int do_trivial = 0; int do_flush = 0; @@ -909,8 +909,6 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, total_response_len += payload.len; } - kh_release_str(shown); - pthread_mutex_lock(&state->main_lock); if (token_data->client_ref_count > 0) @@ -954,6 +952,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, trace2_data_intmax("fsmonitor", the_repository, "response/count/duplicates", duplicates); cleanup: + kh_destroy_str(shown); strbuf_release(&response_token); strbuf_release(&requested_token_id); strbuf_release(&payload); base-commit: 7c7698a654a7a0031f65b0ab0c1c4e438e95df60 -- gitgitgadget