DFS block cache: harden against race over ref locks.

With change https://git.eclipse.org/r/c/jgit/jgit/+/186455 a thread
loading a bitmap index could hold two ref locks at the same time (one
for bitmap and one for either index or reverse index). So it is possible
that two threads loading bitmaps end up in a deadlock for ref locks e.g.
threadA has refLock[1] (for bitmap) and wants refLock[2] (for index or
revIndex) and threadB has refLock[2] (for bitmap) and wants refLock[1].

This change introduces separate pools of locks per pack extension
instead of a shared pool. So threads loading bitmap can hold two
locks but with different extensions and no overlap, e.g. threadA holds
refLock[BITMAP_INDEX][1] and refLock[INDEX][2] and threadB holds
refLock[BITMAP_INDEX][2] and refLock[INDEX][1].

More unit tests were added to cover various paralell loading scenarios.

Signed-off-by: Alina Djamankulova <adjama@google.com>
Change-Id: I89704b4721c21548929608d3798ef60925280755

1 files changed, 12 insertions, 7 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java
index e87bfe24e6..54c527c03c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java
@@ -104,9 +104,10 @@ public final class DfsBlockCache {
 	private final ReentrantLock[] loadLocks;
 
 	/**
-	 * A separate pool of locks to prevent concurrent loads for same index or bitmap from PackFile.
+	 * A separate pool of locks per pack extension to prevent concurrent loads
+	 * for same index or bitmap from PackFile.
 	 */
-	private final ReentrantLock[] refLocks;
+	private final ReentrantLock[][] refLocks;
 
 	/** Maximum number of bytes the cache should hold. */
 	private final long maxBytes;
@@ -173,13 +174,16 @@ public final class DfsBlockCache {
 		}
 
 		table = new AtomicReferenceArray<>(tableSize);
-		loadLocks = new ReentrantLock[cfg.getConcurrencyLevel()];
+		int concurrencyLevel = cfg.getConcurrencyLevel();
+		loadLocks = new ReentrantLock[concurrencyLevel];
 		for (int i = 0; i < loadLocks.length; i++) {
 			loadLocks[i] = new ReentrantLock(true /* fair */);
 		}
-		refLocks = new ReentrantLock[cfg.getConcurrencyLevel()];
-		for (int i = 0; i < refLocks.length; i++) {
-			refLocks[i] = new ReentrantLock(true /* fair */);
+		refLocks = new ReentrantLock[PackExt.values().length][concurrencyLevel];
+		for (int i = 0; i < PackExt.values().length; i++) {
+			for (int j = 0; j < concurrencyLevel; ++j) {
+				refLocks[i][j] = new ReentrantLock(true /* fair */);
+			}
 		}
 
 		maxBytes = cfg.getBlockLimit();
@@ -636,7 +640,8 @@ public final class DfsBlockCache {
 	}
 
 	private ReentrantLock lockForRef(DfsStreamKey key) {
-		return refLocks[(key.hash >>> 1) % refLocks.length];
+		int slot = (key.hash >>> 1) % refLocks[key.packExtPos].length;
+		return refLocks[key.packExtPos][slot];
 	}
 
 	private static AtomicLong[] newCounters() {