Re: [PATCH] packfile: close multi-pack-index in close_all_packs

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Oct 29, 2018, 11:10 UTC
Message-ID: <20181029111041.GQ30222@szeder.dev>
In-Reply-To: <20181025125405.30351-1-dstolee@microsoft.com>

On Thu, Oct 25, 2018 at 12:54:05PM +0000, Derrick Stolee wrote:

Show 23 quoted lines

> Whenever we delete pack-files from the object directory, we need
> to also delete the multi-pack-index that may refer to those
> objects. Sometimes, this connection is obvious, like during a
> repack. Other times, this is less obvious, like when gc calls
> a repack command and then does other actions on the objects, like
> write a commit-graph file.
> 
> The pattern we use to avoid out-of-date in-memory packed_git
> structs is to call close_all_packs(). This should also call
> close_midx(). Since we already pass an object store to
> close_all_packs(), this is a nicely scoped operation.
> 
> This fixes a test failure when running t6500-gc.sh with
> GIT_TEST_MULTI_PACK_INDEX=1.
> 
> Reported-by: Szeder Gábor <szeder.dev@gmail.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> 
> Thanks for the report, Szeder! I think this is the correct fix,
> and more likely to be permanent across all builtins that run
> auto-GC. I based it on ds/test-multi-pack-index so it could easily
> be added on top.

OK, avoiding those errors in the first place is good, of course... However, I still find it disconcerting that those errors didn't cause 'git gc' to error out, and, consequently, what other MIDX-related errors/bugs might do the same.

Show 26 quoted lines

> -Stolee
> 
>  packfile.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/packfile.c b/packfile.c
> index 841b36182f..37fcd8f136 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -339,6 +339,11 @@ void close_all_packs(struct raw_object_store *o)
>  			BUG("want to close pack marked 'do-not-close'");
>  		else
>  			close_pack(p);
> +
> +	if (o->multi_pack_index) {
> +		close_midx(o->multi_pack_index);
> +		o->multi_pack_index = NULL;
> +	}
>  }
>  
>  /*
> 
> base-commit: 0465a50506023df0932fe0534fe6ac6712c0d854
> -- 
> 2.17.1
>