motiejus
/
jgit
1
Fork 0
Code Releases Activity

Revert "RefDirectory: Throw exception if CAS of packed ref list fails" 

This reverts commit 9c33f7364d.

Reason for revert: This change was based on the false claim that the
packedrefs file lock is held while the CAS is being done, but it is
actually released before the CAS (the in memory lock is still held,
however that does not prevent external actors from updating the
packedrefs files and then another thread from subsequently re-reading it
and updating the in memory packedRefList). Although reverting this
change can cause the CAS to fail, it should not actually matter since
the failure would indicate that another thread has already updated the
in memory packedRefList to either the same version this thread was
trying to update it too, or to a more recent version. Either way,
failing the CAS is then appropriate and should not be problematic.

Although this change reverts the code in the RefDirectory class, it
keeps the "improvements" to the test so that it continues to pass
reliably. The reason for the quotes around the word "improvements" is
because I believe the test alteration actually dramatically changes the
intent of the test, and that the original intent of the test is
untestable with the GC and RefDirectory classes as is.

Change-Id: I3acee7527bb542996dcdfaddfb2bdb45ec444db5
Signed-off-by: Martin Fick <quic_mfick@quicinc.com>
This commit is contained in:
Martin Fick 2023-03-08 08:34:38 -07:00
parent 5ae8d28faa
commit c5617711a1
1 changed files with 1 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
View File

@ -1088,22 +1088,7 @@ protected void writeFile(String name, byte[] content)
byte[] digest = Constants.newMessageDigest().digest(content);
PackedRefList newPackedList = new PackedRefList(
refs, lck.getCommitSnapshot(), ObjectId.fromRaw(digest));
// This thread holds the file lock, so no other thread or process should
// be able to modify the packed-refs file on disk. If the list changed,
// it means something is very wrong, so throw an exception.
//
// However, we can't use a naive compareAndSet to check whether the
// update was successful, because another thread might _read_ the
// packed refs file that was written out by this thread while holding
// the lock, and update the packedRefs reference to point to that. So
// compare the actual contents instead.
PackedRefList afterUpdate = packedRefs.updateAndGet(
p -> p.id.equals(oldPackedList.id) ? newPackedList : p);
if (!afterUpdate.id.equals(newPackedList.id)) {
throw new ObjectWritingException(
MessageFormat.format(JGitText.get().unableToWrite, name));
}
packedRefs.compareAndSet(oldPackedList, newPackedList);
if (changed) {
modCnt.incrementAndGet();
}