On Wed, Jul 23, 2008 at 12:41:08AM +0000, Shawn O. Pearce wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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.
Like I said, I had a core that I stupidly lost, but I remember that
the broken malloc was:
static void *get_data_from_pack(struct object_entry *obj)
{
off_t from = obj[0].idx.offset + obj[0].hdr_size;
unsigned long len = obj[1].idx.offset - from;
unsigned long rdy = 0;
unsigned char *src, *data;
z_stream stream;
int st;
src = xmalloc(len);
^^^^^^^^^^^^^^^^^^
len was horribly big, and outputing obj[1].idx showed that `sha1` had
text in it. I mean something like "could not\r\n han" IIRC.
I don't remember the rest of the backtrace, and have stupidly not kept
any ways of reproducing it.
Attachment:
pgp4CbL7t647K.pgp
Description: PGP signature
