Re: Bug: fatal: Unable to create '.../.git/index.lock': File exists.

From: Duy Nguyen <pclouds@gmail.com>
Date: May 3, 2019, 10:22 UTC
Message-ID: <CACsJy8BcBk6Fsfy7o1aBTQAPeH-w+rBwLZrzw4jJ-OM_jhkxMQ@mail.gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.1905031146490.45@tvgsbejvaqbjf.bet>

On Fri, May 3, 2019 at 4:47 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 10 quoted lines

>
> Hi Duy,
>
> On Fri, 3 May 2019, Duy Nguyen wrote:
>
> > I have a feeling that most operations read the index unlocked,
> > manipulate and only lock before writing things out. So yeah it's
> > probably already racy.
>
> IIRC there is a check for that, so it is not actually racy ;-)

Yeah the update_if_able(), used exclusively for refreshing index. My feeling was wrong actually. Looking around a bit, I think we do take the lock, re-read the index, do things, then write.

There may be racy spots still. Looking quickly through some write_locked_index callsites, difftool.c and checkout-index.c may leave a gap between loading the index and locking it. Or refresh_and_write_cache() and a couple others in am.c do not look exactly race-free.

We probably should provide an API where locking requires re-reading the index. The version without re-reading has a big fat warning about danger and stuff.

In any case, i'm getting off topic. I'll stop here.

-- 
Duy