On Fri, Oct 14, 2011 at 09:06:11AM +0200, Johannes Sixt wrote: > Am 10/14/2011 3:23, schrieb Jeff King: > > In practice, however, adding this check still has value, for > > three reasons. > > > > 1. If you have a reasonable number of packs and/or a > > reasonable file descriptor limit, you can keep all of > > your packs open simultaneously. If this is the case, > > then the race is impossible to trigger. > > On Windows, we cannot remove files that are open. If I understand > correctly, this patch keeps more files open for a longer time. Is there > any chance that packfiles remain now open until an unlink() call? > > I am not worried about parallel processes (we already have a problem > there), but that this can now happen within a single process, i.e., that a > single git-repack -a -d -f would now try to unlink a pack file that it > opened itself and did not close timely. > > I'll test your patch later this weekend to see whether the test suite > finds something. But perhaps you know the answer already? With two parallel processes, this will definitely increase the likelihood of a deleted file being open. That is the point. :) Within a single process, I don't think so. This change impacts only pack-objects, which always runs as a separate process, and never deletes packs itself. The most likely problematic code path would be "git repack -d", but it waits for pack-objects to complete successfully before removing any packs. -Peff -- 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