Re: [PATCH 12/26] split-index: fix memory leak in `move_cache_to_base_index()`

["Rubén Justo" <rjusto@gmail.com>]

On Wed, Nov 06, 2024 at 04:10:52PM +0100, Patrick Steinhardt wrote:
> In `move_cache_to_base_index()` we move the index cache of the main
> index into the split index, which is used when writing a shared index.
> But we don't release the old split index base in case we already had a
> split index before this operation, which can thus leak memory.
> 
> Plug the leak by releasing the previous base.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  split-index.c          | 6 +++++-
>  t/t1700-split-index.sh | 1 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/split-index.c b/split-index.c
> index 120c8190b18..cfbc773e6cd 100644
> --- a/split-index.c
> +++ b/split-index.c
> @@ -97,7 +97,11 @@ void move_cache_to_base_index(struct index_state *istate)
>  		mem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool);
>  	}
>  
> -	ALLOC_ARRAY(si->base, 1);
> +	if (si->base)
> +		release_index(si->base);
> +	else
> +		ALLOC_ARRAY(si->base, 1);
> +
>  	index_state_init(si->base, istate->repo);

This change sounded strange to me.  It's like:

   if (si-base) {
   	discard_index(si->base);
   } else {
   	ALLOC_ARRAY(si->base, 1);
   	index_state_init(si->base, istate->repo);
   }

OK.

>  	si->base->version = istate->version;
>  	/* zero timestamp disables racy test in ce_write_index() */
> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> index ac4a5b2734c..2625ac44889 100755
> --- a/t/t1700-split-index.sh
> +++ b/t/t1700-split-index.sh
> @@ -5,6 +5,7 @@ test_description='split index mode tests'
>  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>  
>  # We need total control of index splitting here
> -- 
> 2.47.0.229.g8f8d6eee53.dirty
>