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
deleted.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
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