PackedBatchRefUpdate: Release locks in reverse order of acquisition

PackedBatchRefUpdate#execute takes three locks on the way in. First the
in-process packed-refs lock, then the loose ref locks, and last is the
packed-refs lock file. On the way out it unlocked them in a different
order, the loose refs, then the packed-refs lock file, then the
in-process lock.

This change gives each lock its own finally block, scoped to the region
where it is actually held. Every lock is now unlocked in the reverse of
the order it was locked. And no path can slip past an unlock, since a
failure to release one lock still leaves the enclosing finally blocks to
release the others. Before this, a failure while unlocking the loose
refs would have left the packed-refs lock file behind on disk.

Change-Id: Iac7425f9e4fab666b0c04fc6f153789d24950009
Signed-off-by: Kaushik Lingarkar <klingarkar@nvidia.com>

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java

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 cd03600..12a7031 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
@@ -146,49 +146,52 @@ public void execute(RevWalk walk, ProgressMonitor monitor,
 			return;
 		}
 
-		Map<String, LockFile> locks = null;
-		LockFile packedRefsLock = null;
 		refdb.inProcessPackedRefsLock.lock();
 		try {
-			// Pack refs normally, so we can create lock files even in the case
-			// where refs/x is deleted and refs/x/y is created in this batch.
-			refdb.pack(pending.stream().map(ReceiveCommand::getRefName)
-					.collect(toList()));
-
-			// 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;
-				}
-				refdb.pack(locks);
-			}
-
-			packedRefsLock = refdb.lockPackedRefsOrThrow();
-			PackedRefList oldPackedList = refdb.getLockedPackedRefs(packedRefsLock);
-			RefList<Ref> newRefs = applyUpdates(walk, oldPackedList, pending);
-			if (newRefs == null) {
-				return;
-			}
-			refdb.commitPackedRefs(packedRefsLock, newRefs, oldPackedList,
-					true, oldPackedList.traits());
-		} catch (LockFailedException e) {
-			lockFailure(pending.get(0), pending);
-			return;
-		} finally {
+			Map<String, LockFile> locks = null;
 			try {
-				unlockAll(locks);
-				if (packedRefsLock != null) {
-					// This will be no-op if commitPackedRefs is successful as
-					// it will remove the lock file (by renaming over real
-					// file).
+				// Pack refs normally, so we can create lock files even in
+				// the case where refs/x is deleted and refs/x/y is created
+				// in this batch.
+				refdb.pack(pending.stream().map(ReceiveCommand::getRefName)
+						.collect(toList()));
+
+				// 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;
+					}
+					refdb.pack(locks);
+				}
+
+				LockFile packedRefsLock = refdb.lockPackedRefsOrThrow();
+				try {
+					PackedRefList oldPackedList = refdb
+							.getLockedPackedRefs(packedRefsLock);
+					RefList<Ref> newRefs = applyUpdates(walk, oldPackedList,
+							pending);
+					if (newRefs == null) {
+						return;
+					}
+					refdb.commitPackedRefs(packedRefsLock, newRefs,
+							oldPackedList, true, oldPackedList.traits());
+				} finally {
+					// This will be no-op if commitPackedRefs is successful
+					// as it will remove the lock file (by renaming over
+					// real file).
 					packedRefsLock.unlock();
 				}
+			} catch (LockFailedException e) {
+				lockFailure(pending.get(0), pending);
+				return;
 			} finally {
-				refdb.inProcessPackedRefsLock.unlock();
+				unlockAll(locks);
 			}
+		} finally {
+			refdb.inProcessPackedRefsLock.unlock();
 		}
 
 		refdb.fireRefsChanged();