Re: git rebase -i failing due to phantom index.lock

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Sep 21, 2015, 16:48 UTC
Message-ID: <CAOxFTcx4SsCSbnZ=mv2s7GyDX29gBrZa+uijtbrUTMWjVXXCKg@mail.gmail.com>
In-Reply-To: <CACsJy8CnxGSJT62FRsZP4cD7Az36XGDjJwr+Szddhz42P_hJDg@mail.gmail.com>

Hello,

On Thu, Sep 17, 2015 at 2:57 PM, Duy Nguyen <pclouds@gmail.com> wrote:

Show 35 quoted lines

> On Thu, Sep 17, 2015 at 3:08 AM, Giuseppe Bilotta
> <giuseppe.bilotta@gmail.com> wrote:
>
>> A somewhat problematic git bisect has allowed me to identify commit
>> 03b86647722f11ccc321cd7279aa49b811d17cc2 as the first bad commit.
>
> That commit is mostly refactoring, but there's one extra lock added in
> these hunks. Maybe you can revert it and see if it improves anything.
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 87439fa..5e13444 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -3644,7 +3644,7 @@ static void build_fake_ancestor(struct patch
> *list, const char *filename)
>  {
>         struct patch *patch;
>         struct index_state result = { NULL };
> -       int fd;
> +       static struct lock_file lock;
>
>         /* Once we start supporting the reverse patch, it may be
>          * worth showing the new sha1 prefix, but until then...
> @@ -3682,8 +3682,8 @@ static void build_fake_ancestor(struct patch
> *list, const char *filename)
>                         die ("Could not add %s to temporary index", name);
>         }
>
> -       fd = open(filename, O_WRONLY | O_CREAT, 0666);
> -       if (fd < 0 || write_index(&result, fd) || close(fd))
> +       hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
> +       if (write_locked_index(&result, &lock, COMMIT_LOCK))
>                 die ("Could not write temporary index to %s", filename);
>
>         discard_index(&result);

This is not trivial to revert because write_index isn't available anymore. I'm not sure what I should replace it with.

Show 7 quoted lines

>> The problem has all signs of a timing issue, which I'm having problems
>> isolating, although simply providing a timeout on the index lock calls
>> seems to fix it.
>
> lockfile has received some updates lately. This commit caught my eyes,
> 044b6a9 (lockfile: allow file locking to be retried with a timeout -
> 2015-05-11), but this is just a shot in the dark..

The point is that, if I add a timeout to the checkout lock, it works. It works as if something locks it earlier, and the unlock doesn't register quickly enough, so waiting a few tens of milliseconds makes it work.

> So it fails at "git checkout". That'll give me something to look in
> git-rebase*.sh. Thanks.

For what it's worth, the problem seems easier to replicate with a very long list of commits than with a short one.

Best regards,

-- 
Giuseppe "Oblomov" Bilotta