From 439512d7f82c503638e6b12689e751bb8debc015 Mon Sep 17 00:00:00 2001
From: Ivan Frade <ifrade@google.com>
Date: Wed, 10 Apr 2024 10:52:44 -0700
Subject: [PATCH] DfsPackFile: Do not set local reverse index ref from cache
 callback

The DfsBlockCache loading callback sets the local reference to the
index in the DfsPackFile. This prevents abstracting the loading to
implement it over multiple backends.

Reorg the code so the loadReverseIndex do only the loading, the caller
sets it into DfsBlockCache and the external code sets the local
reference in DfsPackFile.

This is the same pattern we did with the PackIndex in the parent
commit.

Change-Id: I3a395b347965fa7b0e5a3398c4da311dc11c58a1
---
 .../internal/storage/dfs/DfsPackFile.java     | 38 ++++++++++---------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
index b94a84a41c..cdd10618e0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
@@ -330,18 +330,27 @@ public final class DfsPackFile extends BlockBasedFile {
 
 		PackIndex idx = idx(ctx);
 		DfsStreamKey revKey = desc.getStreamKey(REVERSE_INDEX);
-		AtomicBoolean cacheHit = new AtomicBoolean(true);
-		DfsBlockCache.Ref<PackReverseIndex> revref = cache.getOrLoadRef(revKey,
-				REF_POSITION, () -> {
-					cacheHit.set(false);
-					return loadReverseIdx(ctx, revKey, idx);
+		// Keep the value parsed in the loader, in case the Ref<> is
+		// nullified in ClockBlockCacheTable#reserveSpace
+		// before we read its value.
+		AtomicReference<PackReverseIndex> loadedRef = new AtomicReference<>(
+				null);
+		DfsBlockCache.Ref<PackReverseIndex> cachedRef = cache
+				.getOrLoadRef(revKey, REF_POSITION, () -> {
+					RefWithSize<PackReverseIndex> ridx = loadReverseIdx(ctx,
+							idx);
+					loadedRef.set(ridx.ref);
+					return new DfsBlockCache.Ref<>(revKey, REF_POSITION,
+							ridx.size, ridx.ref);
 				});
-		if (cacheHit.get()) {
+		if (loadedRef.get() == null) {
 			ctx.stats.ridxCacheHit++;
 		}
-		PackReverseIndex revidx = revref.get();
-		if (reverseIndex == null && revidx != null) {
-			reverseIndex = revidx;
+		reverseIndex = cachedRef.get() != null ? cachedRef.get()
+				: loadedRef.get();
+		if (reverseIndex == null) {
+			throw new IOException(
+					"Couldn't get a reference to the reverse index"); //$NON-NLS-1$
 		}
 		ctx.emitIndexLoad(desc, REVERSE_INDEX, reverseIndex);
 		return reverseIndex;
@@ -1241,18 +1250,13 @@ public final class DfsPackFile extends BlockBasedFile {
 		}
 	}
 
-	private DfsBlockCache.Ref<PackReverseIndex> loadReverseIdx(
-			DfsReader ctx, DfsStreamKey revKey, PackIndex idx) {
+	private static RefWithSize<PackReverseIndex> loadReverseIdx(DfsReader ctx,
+			PackIndex idx) {
 		ctx.stats.readReverseIdx++;
 		long start = System.nanoTime();
 		PackReverseIndex revidx = PackReverseIndexFactory.computeFromIndex(idx);
-		reverseIndex = revidx;
 		ctx.stats.readReverseIdxMicros += elapsedMicros(start);
-		return new DfsBlockCache.Ref<>(
-				revKey,
-				REF_POSITION,
-				idx.getObjectCount() * 8,
-				revidx);
+		return new RefWithSize<>(revidx, idx.getObjectCount() * 8);
 	}
 
 	private DfsBlockCache.Ref<PackObjectSizeIndex> loadObjectSizeIndex(
-- 
2.39.5