[prev in list] [next in list] [prev in thread] [next in thread]
List: git
Subject: Re: [PATCH v2 0/8] Multiple simultaneously locked ref updates
From: Michael Haggerty <mhagger () alum ! mit ! edu>
Date: 2013-08-31 19:02:46
Message-ID: 52223DD6.3070303 () alum ! mit ! edu
[Download RAW message or body]
On 08/30/2013 08:11 PM, Brad King wrote:
> Here is the second revision of a series to support locking multiple
> refs at the same time to update all of them consistently. The first
> series can be found at $gmane/233260. This revision is ready to
> consider for integration.
I'm very interested in this area and I regret that I have been so
invisible lately. I definitely like the way your changes are going.
I have been doing some work in the same neighborhood and our work
overlaps somewhat. Namely, it is incorrect that delete_ref() currently
deletes the loose ref before rewriting packed-refs, because it can cause
a simultaneous reader to see the packed value for a moment in time, and
the packed value might not even point to a valid object anymore. In
fact, the current version is even worse: it deletes the loose reference
before even locking the packed-refs file, so if the packed refs file
cannot be rewritten (e.g., because it is locked by another process) then
the reference is permanently left in a corrupt state.
On the other hand, writing the packed-refs file first would also be
incorrect, because a pack-refs process could jump in before the loose
refs file was deleted, read the loose value, and write it to the new
packed-refs file. The result would be that the first process would
think that it had deleted the reference but it would still exist (not
such a catastrophe, but incorrect nevertheless).
The solution that I have been working on is to first lock *both* the
loose and packed refs files, then rewrite the packed-refs file *but
retain a lock on it*, then rewrite the loose-ref file and release its
lock, then release the lock on the packed-refs file. By retaining the
lock on the packed-refs file during the whole "transaction", we prevent
another process from trying to pack the refs before our reference is
completely deleted. This requires the file-locking API to be enhanced
to allow a file to be replaced by its new version while still retaining
a lock on the file.
Your code has the same bug as the original (it's not your fault!) so I
think it will eventually have to be fixed to look something like
acquire lock on packed-refs
for reference in ref_updates:
lock reference
if old sha1 known:
verify old sha1 is still current
for reference in ref_updates:
if reference should be created/modified:
modify reference
release lock on reference
delete references from packed-refs file and activate new
version of the file *but retain a lock on the
packed-refs file*
for reference in ref_updates:
if reference should be deleted:
delete loose version of reference
release lock on reference
release lock on packed-refs file
This is really all just for your information; there is certainly no
obligation for you to fix this pre-existing problem. And I'm working on
it anyway; if you happen to be interested you can view my current
work-in-progress on GitHub (though it still doesn't work!):
https://github.com/mhagger/git/tree/WIP-delete-ref-locking
Feedback would of course be welcome.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic