On Tue, Aug 30, 2011 at 03:45:38PM +0200, Nicolas Morey-Chaisemartin wrote:
> According to gdb the problem originate from release_delta_cash (sha1_file.c:1703)
> free(ent->data);
>
> From my analysis it seems that git grep threads do acquire lock before
> calling read_sha1_file but not before calling
> read_object_with_reference who ends up calling read_sha1_file too.
Yeah, I think this is necessary, and the patch looks good.
I notice there are some other code paths that end up in xmalloc without
locking, too (e.g., load_file, and some strbuf_* calls). Don't those
need locking, too, as malloc may try to release packfile memory?
builtin/pack-objects.c dealt with this already by setting a new
"try_to_free" routine that locks[1], which we should also do. It
probably comes up less frequently, because it only happens when we're
under memory pressure.
-Peff
[1] Actually, it looks like the "try_to_free" routine starts as nothing,
and then add_packed_git sets it lazily to try_to_free_pack_memory.
But what builtin/pack-objects tries to do is overwrite that with a
version of try_to_free_pack_memory that does locking. So it's
possible that we would not have read any packed objects while
setting up the threads, and add_packed_git will overwrite our
careful, locking version of try_to_free_pack_memory.
I _think_ pack-objects is probably OK, because it will have already
done the complete "counting objects" phase, which would look in any
packs. But it may be harder for grep.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html