Re: git rev-list | git cherry-pick --stdin is leaky

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Am 30.04.2013 02:11, schrieb Stephen Boyd:
> (resending since the attachment seems to make vger sad)
> 
> Hi,
> 
> I'm running git rev-list | git cherry-pick --stdin on a range of about
> 300 commits. Eventually the chery-pick dies with:
> 
>      error: cannot fork() for commit: Cannot allocate memory
> 
> Running valgrind shows me that the tree traversal code is leaking
> gigabytes of memory (particularly unpack_callback). Since cherry-pick is
> a very long running process all these allocations are never freed and
> eventually I run out of memory. The worst offender and summary is:
> 
> ==7986== 938,956,692 (929,961,582 direct, 8,995,110 indirect) bytes in
> 7,765,439 blocks are definitely lost in loss record 257 of 257
> ==7986==    at 0x4C267CC: calloc (vg_replace_malloc.c:467)
> ==7986==    by 0x4FAF57: xcalloc (wrapper.c:119)
> ==7986==    by 0x4F5281: unpack_callback (unpack-trees.c:539)
> ==7986==    by 0x4F40E5: traverse_trees (tree-walk.c:407)
> ==7986==    by 0x4F586C: unpack_callback (unpack-trees.c:467)
> ==7986==    by 0x4F40E5: traverse_trees (tree-walk.c:407)
> ==7986==    by 0x4F586C: unpack_callback (unpack-trees.c:467)
> ==7986==    by 0x4F40E5: traverse_trees (tree-walk.c:407)
> ==7986==    by 0x4F586C: unpack_callback (unpack-trees.c:467)
> ==7986==    by 0x4F40E5: traverse_trees (tree-walk.c:407)
> ==7986==    by 0x4F586C: unpack_callback (unpack-trees.c:467)
> ==7986==    by 0x4F40E5: traverse_trees (tree-walk.c:407)
> ==7986==
> ==7986== LEAK SUMMARY:
> ==7986==    definitely lost: 2,514,117,692 bytes in 21,210,861 blocks
> ==7986==    indirectly lost: 885,481,947 bytes in 10,165,801 blocks
> ==7986==      possibly lost: 650,712,395 bytes in 6,014,309 blocks
> ==7986==    still reachable: 7,734,870 bytes in 47,794 blocks
> ==7986==         suppressed: 0 bytes in 0 blocks

I looked at that particular leak a year ago but couldn't convince myself
to submit the patch below.  If the callback function we call through
call_unpack_fn does something strange like free()ing entries itself or
adding them to some list without duplication then the added free() can
cause trouble.

Looking at it again today I don't understand that concern any more.  The
current callback functions don't do something like that, in any case.
Maybe I'm missing something.

Anyway, could you please check if the patch helps with your use case?

Thanks,
René


diff --git a/unpack-trees.c b/unpack-trees.c
index ede4299..67bc698 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -590,8 +590,13 @@ static int unpack_nondirectories(int n, unsigned long mask,
 		src[i + o->merge] = create_ce_entry(info, names + i, stage);
 	}
 
-	if (o->merge)
-		return call_unpack_fn(src, o);
+	if (o->merge) {
+		int ret = call_unpack_fn(src, o);
+		for (i = 1; i < n + 1; i++)
+			if (src[i] && src[i] != o->df_conflict_entry)
+				free(src[i]);
+		return ret;
+	}
 
 	for (i = 0; i < n; i++)
 		if (src[i] && src[i] != o->df_conflict_entry)