During a transaction commit we will both update and delete refs.
Since both update and delete now use the same pattern

    lock = lock_ref_sha1_basic() (or varient of)
    write_ref_sha1(lock)/delete_ref_loose(lock)
    unlock_ref(lock) | commit_ref_lock(lock)

we can now simplify ref_transaction_commit to have one loop that locks all
involved refs.
A second loop that writes or flags for deletion, but does not commit, all
the refs.
And a final third loop that commits all the refs once all the work and
preparations are complete.

This makes updating/deleting multiple refs more atomic since we will not start
the commit phase until all the preparations have completed successfully.

Signed-off-by: Ronnie Sahlberg <sahlb...@google.com>
---
 refs.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/refs.c b/refs.c
index ec104f2..293bb43 100644
--- a/refs.c
+++ b/refs.c
@@ -3468,42 +3468,47 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
                }
        }
 
-       /* Perform updates first so live commits remain referenced */
+       /* Prepare all the updates/deletes */
        for (i = 0; i < n; i++) {
                struct ref_update *update = updates[i];
 
-               if (!is_null_sha1(update->new_sha1)) {
+               if (!is_null_sha1(update->new_sha1))
                        ret = update_ref_write(msg,
                                               update->refname,
                                               update->new_sha1,
                                               update->lock, onerr);
-                       if (ret)
-                               unlock_ref(update->lock);
-                       else
-                               commit_ref_lock(update->lock);
-                       update->lock = NULL;
-                       if (ret)
-                               goto cleanup;
+               else {
+                       delnames[delnum++] = update->refname;
+                       ret = delete_ref_loose(update->lock, update->type);
                }
+               if (ret)
+                       goto cleanup;
        }
 
-       /* Perform deletes now that updates are safely completed */
+       ret |= repack_without_refs(delnames, delnum);
+       for (i = 0; i < delnum; i++)
+               unlink_or_warn(git_path("logs/%s", delnames[i]));
+       clear_loose_ref_cache(&ref_cache);
+
+       /* Perform updates first so live commits remain referenced */
+       for (i = 0; i < n; i++) {
+               struct ref_update *update = updates[i];
+
+               if (update->lock && !update->lock->delete_ref) {
+                       ret |= commit_ref_lock(update->lock);
+                       update->lock = NULL;
+               }
+       }
+       /* And finally perform all deletes */
        for (i = 0; i < n; i++) {
                struct ref_update *update = updates[i];
 
                if (update->lock) {
-                       delnames[delnum++] = update->refname;
-                       ret |= delete_ref_loose(update->lock, update->type);
                        ret |= commit_ref_lock(update->lock);
                        update->lock = NULL;
                }
        }
 
-       ret |= repack_without_refs(delnames, delnum);
-       for (i = 0; i < delnum; i++)
-               unlink_or_warn(git_path("logs/%s", delnames[i]));
-       clear_loose_ref_cache(&ref_cache);
-
 cleanup:
        for (i = 0; i < n; i++)
                if (updates[i]->lock)
-- 
1.9.1.505.g4f1e74f

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

Reply via email to