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 other/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
s/existsing/existing/
> 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()
message),
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html