During update_refs we will both be deleting refs as well as updating/changing
the refs.
Since both delete and update now use the same lock/update/commit 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 update_update refs 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 | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/refs.c b/refs.c
index 038614d..e94099b 100644
--- a/refs.c
+++ b/refs.c
@@ -3369,33 +3369,31 @@ int update_refs(const char *action, const struct 
ref_update **updates_orig,
        }
 
        /* Perform updates first so live commits remain referenced */
-       for (i = 0; i < n; i++)
-               if (!is_null_sha1(updates[i]->new_sha1)) {
+       for (i = 0; i < n; i++) {
+               if (!is_null_sha1(updates[i]->new_sha1))
                        ret = update_ref_write(action,
                                               updates[i]->ref_name,
                                               updates[i]->new_sha1,
                                               locks[i], onerr);
-                       if (ret)
-                               unlock_ref(locks[i]);
-                       else
-                               commit_ref_lock(locks[i]);
-                       locks[i] = NULL;
-                       if (ret)
-                               goto cleanup;
+               else {
+                       delnames[delnum++] = locks[i]->ref_name;
+                       ret = delete_ref_loose(locks[i], types[i]);
                }
+               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);
+
+       /* Commit all the updates/deletions */
        for (i = 0; i < n; i++)
                if (locks[i]) {
-                       delnames[delnum++] = locks[i]->ref_name;
-                       ret |= delete_ref_loose(locks[i], types[i]);
                        ret |= commit_ref_lock(locks[i]);
                        locks[i] = 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++)
-- 
1.9.1.477.g7668a0d.dirty

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