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. -Peff