Race condition in git push --mirror can cause silent ref rewinding

Heya,

We recently ran into a particularly troubling race condition, discovered in git 2.0.0. The setup for it is as follows:

The repository is a bare repository, which developers push to via ssh; it mirrors its changes out onto github. In its config:

[remote "github"]
url = [email protected]:bestpractical/rt.git
fetch = +refs/*:refs/*
mirror = yes

It has a post-receive hook which does:

sudo -u git -H /usr/bin/git push github

We recently saw a situation where a push of a new branch caused a simultaneous update of a different branch (by a different user) to be rewound. From the reflog of the created branch (4.2/html-gumbo-loading):

    0000000000000000000000000000000000000000

1aefd600fcbb5ded14376f77d77a14758668fb39 Wallace Reis
<[email protected]> 1404326443 -0400 push

And the updated branch (4.2-trunk), which was rewound:

    44dc8ad0e4603e3f674b7c00deacc122ca52707a
1e743b6225d502ad1a265929fb873f4c0bf4f8a5 Kevin Falcone
<[email protected]> 1404326446 -0400    push
    1e743b6225d502ad1a265929fb873f4c0bf4f8a5

44dc8ad0e4603e3f674b7c00deacc122ca52707a git <[email protected]>
1404326446 -0400 update by push

It is my belief that this comes because the "--mirror" argument causes the local refs to be treated as tracking refs -- and thus updates all of them during the push. I believe the race condition is thus:

1. User A starts a push --mirror; git records the values of the refs

2. User B updates a ref, commit mail goes out, etc

3. User A's push completes, updates "tracking" branch to value at (1).

Needless to say, silently losing commits which appeared for all purposes to be pushed successfully (neither User A nor User B sees anything out of the ordinary) is extremely troubling.

- Alex

"git push github master^:master" must stay a usable way to update the published repository to an arbitrary commit, so "if set to mirror, do not pretend that a fetch in reverse has happened during 'git push'" will not be a solution to this issue.

Perhaps removing remote.github.fetch would be one sane way forward. Otherwise, even if your "git push" does not pretend to immediately fetch from there (i.e. even if the reported behaviour was a bug, without doing anything to trigger it) somebody running "git fetch" in this repository can destroy what other person pushes into this repository at the same time exactly the same way, I would think.

Hm? I'm confused, as mirror isn't compatible with refspecs:

$ git push github master^:master
error: --mirror can't be combined with refspecs

Ahh -- I see. The repository predates a9f5a355, which split git remote add --mirror into --mirror=push and --mirror=fetch, because of more or less this exact problem. Of course, there is nothing much that can be done for existing repositories in this situation as it's a legitimate combination.
- Alex

Just following up on this -- can you clarify your statement about "git push github master^:master" in light of the fact that --mirror already disallows such?
- Alex