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

2015-09-21 Thread Giuseppe Bilotta
Hello,

On Thu, Sep 17, 2015 at 2:57 PM, Duy Nguyen  wrote:
> On Thu, Sep 17, 2015 at 3:08 AM, Giuseppe Bilotta
>  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.

>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-09-17 Thread Duy Nguyen
On Thu, Sep 17, 2015 at 3:08 AM, Giuseppe Bilotta
 wrote:
> Hello all,
>
> I've recently started to note an issue with git rebase -i failing with
> alarming frequency, especially on one of my repositories, claiming
> that index.lock could not be created because it exists, even though it
> doesn't and nothing else seems to be locking the index. The rebase
> bombs more usually during the initial checkout than during any other
> of the steps.
>
> The problem is somewhat randomly reproducible, as it doesn't happen
> 100% of the time. It also seems to happen more consistently with more
> recent git versions, or at least with my custom built git than with
> the distro-shipped one.
>
> 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);

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

> Making git stall instead of die on error allowed me
> to obtain this backtrace which I suspect will not be particularly
> useful:
>
> #0 0x004c4666 in unable_to_lock_message
> (path=path@entry=0x1bad940 ".git/index", err=,
> buf=buf@entry=0x7ffd3b990900) at lockfile.c:158
> #1 0x004c46c6 in unable_to_lock_die (path=path@entry=0x1bad940
> ".git/index", err=) at lockfile.c:167
> #2 0x004c480b in hold_lock_file_for_update_timeout
> (lk=lk@entry=0x1bd7be0, path=0x1bad940 ".git/index", flags= out>, timeout_ms=timeout_ms@entry=0) at lockfile.c:177
> #3 0x004e6e2a in hold_lock_file_for_update (flags=1,
> path=, lk=0x1bd7be0) at lockfile.h:165
> #4 hold_locked_index (lk=lk@entry=0x1bd7be0,
> die_on_error=die_on_error@entry=1) at read-cache.c:1411
> #5 0x00420cb0 in merge_working_tree (old=0x7ffd3b990a20,
> old=0x7ffd3b990a20, new=0x7ffd3b990a00, new=0x7ffd3b990a00,
> writeout_error=0x7ffd3b9909c0, opts=0x7ffd3b992a31)
> at builtin/checkout.c:471

So it fails at "git checkout". That'll give me something to look in
git-rebase*.sh. Thanks.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git rebase -i failing due to phantom index.lock

2015-09-16 Thread Giuseppe Bilotta
Hello all,

I've recently started to note an issue with git rebase -i failing with
alarming frequency, especially on one of my repositories, claiming
that index.lock could not be created because it exists, even though it
doesn't and nothing else seems to be locking the index. The rebase
bombs more usually during the initial checkout than during any other
of the steps.

The problem is somewhat randomly reproducible, as it doesn't happen
100% of the time. It also seems to happen more consistently with more
recent git versions, or at least with my custom built git than with
the distro-shipped one.

A somewhat problematic git bisect has allowed me to identify commit
03b86647722f11ccc321cd7279aa49b811d17cc2 as the first bad commit.

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. Making git stall instead of die on error allowed me
to obtain this backtrace which I suspect will not be particularly
useful:

#0 0x004c4666 in unable_to_lock_message
(path=path@entry=0x1bad940 ".git/index", err=,
buf=buf@entry=0x7ffd3b990900) at lockfile.c:158
#1 0x004c46c6 in unable_to_lock_die (path=path@entry=0x1bad940
".git/index", err=) at lockfile.c:167
#2 0x004c480b in hold_lock_file_for_update_timeout
(lk=lk@entry=0x1bd7be0, path=0x1bad940 ".git/index", flags=, timeout_ms=timeout_ms@entry=0) at lockfile.c:177
#3 0x004e6e2a in hold_lock_file_for_update (flags=1,
path=, lk=0x1bd7be0) at lockfile.h:165
#4 hold_locked_index (lk=lk@entry=0x1bd7be0,
die_on_error=die_on_error@entry=1) at read-cache.c:1411
#5 0x00420cb0 in merge_working_tree (old=0x7ffd3b990a20,
old=0x7ffd3b990a20, new=0x7ffd3b990a00, new=0x7ffd3b990a00,
writeout_error=0x7ffd3b9909c0, opts=0x7ffd3b992a31)
at builtin/checkout.c:471
#6 switch_branches (new=0x7ffd3b990a00, opts=0x7ffd3b992a31) at
builtin/checkout.c:826
#7 checkout_branch (new=0x7ffd3b990a00, opts=0x7ffd3b992a31) at
builtin/checkout.c:1123
#8 cmd_checkout (argc=, argv=,
prefix=) at builtin/checkout.c:1273
#9 0x00405e7e in run_builtin (argv=0x7ffd3b992480, argc=2,
p=0x7acab0 ) at git.c:350
#10 handle_builtin (argc=2, argv=0x7ffd3b992480) at git.c:535
#11 0x00405021 in run_argv (argv=0x7ffd3b9922d8,
argcp=0x7ffd3b9922bc) at git.c:581
#12 main (argc=2, av=) at git.c:689

Additional information: I have the bash git prompt enabled, I do not
have anything else accessing that repository at the same time, and the
repository is “spread out” across multiple workdirs created with git
new-workdir, each with a different branch checked out. It is also
usually accessed under a symlink (cd ~/shortcut/repo/branch rather
than ~/full/path/to/repo/branch). Running Debian unstable with Linux
4.1.0 on an SSD-backed ext4 partition mounted with data=ordered. I
can't thnk of any other detail that might be even just remotely
involved in this odd issue. Any suggestions on how to debug it are
welcome.

Best regards,

-- 
Giuseppe "Oblomov" Bilotta
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html