From 47f2f3613c222c21b47b031316414e09a66c8a0c Mon Sep 17 00:00:00 2001
From: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
Date: Mon, 27 Feb 2023 15:03:32 -0800
Subject: [PATCH] PackedBatchRefUpdate: Ensure updates are applied on latest
 packed refs

In the window between refs being packed (via refDb.pack) and obtaining
updates (via applyUpdates), packed-refs may have been updated by another
actor and relying on the previously read contents may lead to losing the
updates done by the other actor. To help avoid this, read packed-refs
from disk to ensure we have the latest copy after it is locked and
before committing updates to it.

Bug: 581641
Change-Id: Iae71cb30830b307d0df929c9131911ee476c711c
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
---
 .../storage/file/PackedBatchRefUpdate.java    | 32 +++++++++++--------
 .../internal/storage/file/RefDirectory.java   | 13 ++++++--
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java
index 8b0ea4fcd..560b80756 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java
@@ -160,31 +160,35 @@ public void execute(RevWalk walk, ProgressMonitor monitor,
 		Map<String, LockFile> locks = null;
 		refdb.inProcessPackedRefsLock.lock();
 		try {
-			PackedRefList oldPackedList;
+			// During clone locking isn't needed since no refs exist yet.
+			// This also helps to avoid problems with refs only differing in
+			// case on a case insensitive filesystem (bug 528497)
 			if (!refdb.isInClone() && shouldLockLooseRefs) {
 				locks = lockLooseRefs(pending);
 				if (locks == null) {
 					return;
 				}
-				oldPackedList = refdb.pack(locks);
-			} else {
-				// During clone locking isn't needed since no refs exist yet.
-				// This also helps to avoid problems with refs only differing in
-				// case on a case insensitive filesystem (bug 528497)
-				oldPackedList = refdb.getPackedRefs();
-			}
-			RefList<Ref> newRefs = applyUpdates(walk, oldPackedList, pending);
-			if (newRefs == null) {
-				return;
+				refdb.pack(locks);
 			}
+
 			LockFile packedRefsLock = refdb.lockPackedRefs();
 			if (packedRefsLock == null) {
 				lockFailure(pending.get(0), pending);
 				return;
 			}
-			// commitPackedRefs removes lock file (by renaming over real file).
-			refdb.commitPackedRefs(packedRefsLock, newRefs, oldPackedList,
-					true);
+			try {
+				PackedRefList oldPackedList = refdb.refreshPackedRefs();
+				RefList<Ref> newRefs = applyUpdates(walk, oldPackedList, pending);
+				if (newRefs == null) {
+					return;
+				}
+				refdb.commitPackedRefs(packedRefsLock, newRefs, oldPackedList,
+						true);
+			} finally {
+				// This will be no-op if commitPackedRefs is successful as it
+				// will remove the lock file (by renaming over real file).
+				packedRefsLock.unlock();
+			}
 		} finally {
 			try {
 				unlockAll(locks);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
index 065c20b61..72f1d5afa 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
@@ -924,9 +924,18 @@ PackedRefList getPackedRefs() throws IOException {
 			break;
 		}
 
+		return refreshPackedRefs(curList);
+	}
+
+	PackedRefList refreshPackedRefs() throws IOException {
+		return refreshPackedRefs(packedRefs.get());
+	}
+
+	private PackedRefList refreshPackedRefs(PackedRefList curList)
+			throws IOException {
 		final PackedRefList newList = readPackedRefs();
-		if (packedRefs.compareAndSet(curList, newList)
-				&& !curList.id.equals(newList.id)) {
+		if (packedRefs.compareAndSet(curList, newList) && !curList.id.equals(
+				newList.id)) {
 			modCnt.incrementAndGet();
 		}
 		return newList;