On Wed, May 17, 2017 at 6:15 PM, Jeff King <peff@xxxxxxxx> wrote: > On Wed, May 17, 2017 at 10:57:34AM -0700, Stefan Beller wrote: > >> On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote: >> > If we've got the "packed-refs" file locked, then it can't change; >> > there's no need to keep calling `stat_validity_check()` on it. >> >> This change will work in a world where all Git implementations >> obey a lock. If there is at least one implementation that doesn't >> care about the existence of lock files we may introduce a race >> here. >> >> I am not sure if it is worth to be extra careful in the common case >> though. But I could imagine some people using a git repo on an >> NFS concurrently with different implementations and one of them >> is an old / careless lock-ignoring implementation. >> >> My opinion is not strong enough that I'd veto such a patch >> just food for thought. > > You're so unbelievably screwed if somebody is not respecting the lock > that I don't think it's worth considering. > > This change just drops the stat_validity_check(), so you may fail to > realize that somebody racily (without holding the lock!) changed the > packed refs, and may omit a ref from your traversal if it moved from > loose to packed. That _can_ have lasting corruption effects if your > operation is something like "git prune" that is computing full > reachability. > > But even without this change, somebody writing the packed-refs file > without lock is likely to hose over simultaneous writes and lose ref > updates (or even lose refs entirely). So anybody who doesn't respect the > locks is broken, period, and needs to be fixed. ok, in that case I see no objection for this patch. Stefan