Re: regression in 92392b4

["Shawn O. Pearce" <spearce@spearce.org>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 23 Jul 2008, Pierre Habouzit wrote:
> 
> >   Hi, here is a manual painful down-secting (opposed to a bisect ;P) I
> > did, since git in next cannot fetch on a regular basis for me. The
> > culprit seems to be commit  92392b4:
> > 
> >     ┌─(1:11)──<~/dev/scm/git 92392b4...>──
> >     └[artemis] git fetch
> >     remote: Counting objects: 461, done.
> >     remote: Compressing objects: 100% (141/141), done.
> >     remote: Total 263 (delta 227), reused 155 (delta 121)
> >     Receiving objects: 100% (263/263), 95.55 KiB, done.
> >     fatal: Out of memory, malloc failed
> >     fatal: index-pack failed
> >     [2]    16674 abort (core dumped)  git fetch
...
> 
> Just a guess:
...
> diff --git a/index-pack.c b/index-pack.c
> index ac20a46..19c39e5 100644
> --- a/index-pack.c
> +++ b/index-pack.c
> @@ -257,6 +257,7 @@ static void unlink_base_data(struct base_data *c)
>  		base_cache = NULL;
>  	if (c->data) {
>  		free(c->data);
> +		c->data = NULL;
>  		base_cache_used -= c->size;
>  	}
>  }

Oh.  This is a pointless assignment.  If you look at any call sites
for unlink_base_data() you will find that the struct passed in as
"c" here is going out of scope after unlink_base_data() returns.  In
no such case does the value of c->data get tested once this free is
complete.

We need the if (c->data) guard because we only want to decrement
base_cache_used if the memory is still allocated.  It may have been
released earlier, in which case base_cache_used has already been
decreased and we don't want to double-decrement it.

This patch makes the code more obvious, so Ack I guess, but it is
not a solution to Pierre's woes.  Something else is wrong.

Reading above shows we got a "fatal: Out of memory, malloc failed"
right before the segfault.  What's odd is we segfaulted after we
ran out of memory and should have die'd.

There's at least two bugs in the above output:

a) index-pack ran out of memory on a small pull (95 KiB).
b) fetch segfaulted when index-pack failed.

And this patch will unfortunately address neither of them.  :-|

I've had a long past couple of days, and another one tomorrow.
I'm not going to be able to debug this myself until perhaps Thursday
or Friday.  Sorry.  If nobody beats me to it, I will put this on
the top of the pile and try to fix it once I get back online at my
new home.

-- 
Shawn.