On 03/31/2014 07:56 PM, Ronnie Sahlberg wrote:
> I am new to git, so sorry If I overlooked something.
> I think there might be a race in ref_transaction_commit() when
> deleting references.
>       /* Perform deletes now that updates are safely completed */
>       for (i = 0; i < n; i++) {
>               struct ref_update *update = updates[i];
>               if (update->lock) {
>                       delnames[delnum++] = update->lock->ref_name;
>                       ret |= delete_ref_loose(update->lock, update->type);
>               }
>       }
>       ret |= repack_without_refs(delnames, delnum);
>       for (i = 0; i < delnum; i++)
>               unlink_or_warn(git_path("logs/%s", delnames[i]));
> These two blocks should be reordered so that you first delete the
> actual refs first, while holding the lock and then release the lock
> afterward ?

I think what you suggest is what is already being done.  The locks of
references that are being deleted are not released until a few lines
after the code that you quoted:

>       for (i = 0; i < n; i++)
>               if (updates[i]->lock)
>                       unlock_ref(updates[i]->lock);

Before the code that you quoted, some locks are released, but only for
references being updated (not those being deleted).

But maybe I misunderstand your critique.

By the way, there *is* a race here, but it is a subtler one involving
the interaction between packed and loose references when references are


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