On Thu, Sep 28, 2017 at 08:38:39AM +0000, Olga Telezhnaya wrote:
> diff --git a/packfile.c b/packfile.c
> index f69a5c8d607af..ae3b0b2e9c09a 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -876,6 +876,7 @@ void prepare_packed_git(void)
> for (alt = alt_odb_list; alt; alt = alt->next)
> prepare_packed_git_one(alt->path, 0);
> rearrange_packed_git();
> + INIT_LIST_HEAD(&packed_git_mru.list);
> prepare_packed_git_mru();
> prepare_packed_git_run_once = 1;
> }
I was thinking on this hunk a bit more, and I think it's not quite
right.
The prepare_packed_git_mru() function will clear the mru list and then
re-add each item from the packed_git list. But by calling
INIT_LIST_HEAD() here, we're effectively clearing the packed_git_mru
list, and we end up leaking whatever was on the list before.
So for the first call to prepare_packed_git, we really need this
INIT_LIST_HEAD() call. But for subsequent calls (which come from
reprepare_packed_git()), we must not call it.
There are a few ways to work around it that I can think of:
1. Check whether packed_git_mru.list.head is NULL, and only initialize
in that case.
2. Use a static initializer for packed_git_mru.list, so that we don't
have do the first-time initializing here.
3. Teach reprepare_packed_git() to do the mru_clear() call, so that we
know the list is empty when we get here.
One final and more invasive option is to stop regenerating the
packed_git_mru list from scratch during each prepare_packed_git(). I did
it that way so that we start with the same order that
rearrange_packed_git() will give us, but I'm not sure how much value
that has in practice (it probably had a lot more when we didn't have the
mru, and the time-sorted pack order helped find recent objects more
quickly).
The alternative would be to just teach install_packed_git() to add each
newly-added pack to the mru list, and then never clear the list (and we
wouldn't need an mru_clear() at all, then).
-Peff
