[PATCH v3 16/19] branch.c: use ref transaction for all ref updates
From: Ronnie Sahlberg <hidden>
Date: 2016-06-15 23:00:50
Subsystem:
the rest · Maintainer:
Linus Torvalds
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. If the forcing flag is false then use ref_transaction_create since this will fail if the ref already exist. Otherwise use ref_transaction_update. 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. Signed-off-by: Ronnie Sahlberg <redacted> --- branch.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/branch.c b/branch.c
index 660097b..23cde1e 100644
--- a/branch.c
+++ b/branch.c@@ -226,7 +226,6 @@ void create_branch(const char *head, int force, int reflog, int clobber_head, int quiet, enum branch_track track) { - struct ref_lock *lock = NULL; struct commit *commit; unsigned char sha1[20]; char *real_ref, msg[PATH_MAX + 20];
@@ -285,15 +284,6 @@ void create_branch(const char *head, die(_("Not a valid branch point: '%s'."), start_name); hashcpy(sha1, commit->object.sha1); - if (!dont_change_ref) { - lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL); - if (!lock) - die_errno(_("Failed to lock ref for update")); - } - - if (reflog) - log_all_ref_updates = 1; - if (forcing) snprintf(msg, sizeof msg, "branch: Reset to %s", start_name);
@@ -301,13 +291,34 @@ void create_branch(const char *head, snprintf(msg, sizeof msg, "branch: Created from %s", start_name); + if (reflog) + log_all_ref_updates = 1; + + if (!dont_change_ref) { + struct ref_transaction *transaction; + char *err = NULL; + + transaction = ref_transaction_begin(); + if (forcing) { + if (!transaction || + ref_transaction_update(transaction, ref.buf, sha1, + NULL, 0, 0) || + ref_transaction_commit(transaction, msg, &err)) + die_errno(_("%s: failed to write ref: %s"), + ref.buf, err); + } else { + if (!transaction || + ref_transaction_create(transaction, ref.buf, sha1, + 0) || + ref_transaction_commit(transaction, msg, &err)) + die_errno(_("%s: failed to write ref: %s"), + ref.buf, err); + } + } + if (real_ref && track) setup_tracking(ref.buf + 11, real_ref, track, quiet); - if (!dont_change_ref) - if (write_ref_sha1(lock, sha1, msg) < 0) - die_errno(_("Failed to write ref")); - strbuf_release(&ref); free(real_ref); }
--
1.9.1.521.g5dc89fa