Re: [PATCH] perf: work around the tested repo having an index.lock

From: Jeff King <peff@peff.net>
Date: Jun 4, 2017, 08:37 UTC
Message-ID: <20170604083743.kovvk5k43fk4kmur@sigill.intra.peff.net>
In-Reply-To: <xmqqwp8sv77h.fsf@gitster.mtv.corp.google.com>

On Sun, Jun 04, 2017 at 11:04:50AM +0900, Junio C Hamano wrote:

Show 24 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > But I think a more compelling case is that there may be an ongoing
> > operation in the original repo (e.g., say you are in the middle of
> > writing a commit message) when we do a blind copy of the filesystem
> > contents. You might racily pick up a lockfile.
> >
> > Should we find and delete all *.lock files in the copied directory? That
> > would get ref locks, etc. Half-formed object files are OK. Technically
> > if you want to get an uncorrupted repository you'd also want to copy
> > refs before objects (in case somebody makes a new object and updates a
> > ref while you're copying).
> 
> Or "git branch -m A B" is in progress.
> 
> I think it all depends on what your "threat" model is ;-).  Do we
> assume that many users are "time-sharing" a box and a repository?
> If not, i.e. if you are the sole user of a box and a repository on
> it, such a concurrent access to make the result of git-unaware copy
> problematic will not be in index.lock (after all you are now doing
> the perf thing, not editing a commit log message in the repository
> used for testing Git), but will be in ref locks (somebody else
> pushing into the repository you are *not* currently using from
> sideways).

I was specifically thinking of the case where you run "git commit -a", it locks the index, and then while you are writing the commit message you need to collect some more perf results. The default perf repo is the current repository itself, so running any perf script will copy the index.lock and probably cause the script to misbehave.

That doesn't seem like an implausible sequence of events (frankly, I'm surprised I haven't hit it myself, as I often run perf scripts while writing commit messages. I think I've been saved by generally using a separate linux.git clone as my test repo).

So it may be reasonable to say that the index.lock is special. We hold it for a long time (compared to reflocks, which do a quick compare-and-swap). And then we can throw up our hands for any other races. People who run "git prune" on their test repository running the perf scripts get what they deserve. :)

-Peff