Ronnie Sahlberg wrote:

> Change create_branch to use a ref transaction when creating the new branch.
> ref_transaction_create will check that the ref does not already exist and fail
> otherwise meaning that we no longer need to keep a lock on the ref during the
> setup_tracking. This simplifies the code since we can now do the transaction
> in one single step.

Previously the ref lock was also held during setup_tracking, which
sets up configuration for the branch in .git/config.  So in principle
they were one transaction and this patch would change that.

The race:

        checkout -B master origin/master
                                update-ref -d refs/heads/master
                                                        checkout -B master 
        create ref
                                delete ref
                                                        create ref

        configure tracking                              configure tracking

Since setup_tracking is not a single transaction, if the two processes
are lucky enough to not try to write to .git/config at the same time
then the resulting configuration could have branch.master.merge set
by the first checkout -b and branch.master.remote set by the second.

But trying to "checkout -b" the same branch in two terminals
concurrently is a somewhat insane thing to do, so I don't mind
breaking it.

> This also fixes a race condition in the old code where two concurrent
> create_branch could race since the lock_any_ref_for_update/write_ref_sha1
> did not protect against the ref already existsing. I.e. one thread could end 
> up


> overwriting a branch even if the forcing flag is false.

Good catch.

> --- a/branch.c
> +++ b/branch.c
> @@ -301,13 +291,25 @@ void create_branch(const char *head,
> +             if (!transaction ||
> +                     ref_transaction_update(transaction, ref.buf, sha1,
> +                                            null_sha1, 0, !forcing) ||
> +                     ref_transaction_commit(transaction, msg, &err))
> +                             die_errno(_("%s: failed to write ref: %s"),
> +                                       ref.buf, err.buf);

errno is not guaranteed valid here.  The usual

                                die("%s", err.buf);

should do the trick.

With the changes mentioned above (commit message typofix, die()
Reviewed-by: Jonathan Nieder <>

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to