[PATCH 2/2] packfile: recover when a multi-pack-index names a removed pack

From: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date: Aug 18, 2026, 22:34 UTC
Message-ID: <5792c08f4ee0f9627ab1432d91299fe676e0a2f5.1787092446.git.gitgitgadget@gmail.com>
In-Reply-To: <pull.2207.git.1787092446.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

When a geometric repack runs concurrently with other git processes, it can write a new pack and multi-pack-index and then delete older packs that the new one subsumes. One or more of those older packs may have been indexed by the previous multi-pack-index. A process that already had the previous multi-pack-index open keeps using it, and that stale index still records the removed pack(s) as owning some objects.

Because a multi-pack-index attributes each object to exactly one pack, an object that exists in multiple covered packs is served only through its recorded owner. If that owner is the pack a concurrent repack just removed, find_pack_entry() cannot serve the object: fill_midx_entry() routes the lookup to the missing pack (prepare_midx_pack() fails), and the regular pack fallback deliberately skips every multi-pack-index covered pack. The object is reported missing even though a perfectly good copy survives in another covered pack -- for example a large "base" pack that geometric repacking intentionally kept.

The false negative is not limited to one caller. Any reader (cat-file, rev-list, pack-objects, ...) can spuriously fail with "unable to read object", and callers that only ask whether an object exists get a wrong answer too, since the OBJECT_INFO_QUICK path never retries. Writers that merge in-core, such as "git replay", are hit hardest: merge-ort treats the unreadable tree as a premature abort, sets result.clean < 0, and returns without a result tree.

Teach find_pack_entry() to recover. After the normal multi-pack-index lookup and the regular pack fallback both miss, check whether the object is nonetheless present in a covered multi-pack-index (bsearch_midx()). If it is, its recorded owner must have become unavailable, so scan that index's packs directly for a surviving copy. The bsearch gate keeps genuine misses (i.e. objects absent from the index) on the fast path, and because the recovery lives in find_pack_entry() itself it also fixes the OBJECT_INFO_QUICK callers that never reprepare.

This recovers the object without touching the multi-pack-index itself. Reloading the stale index would be a more complete fix but would be much more involved: other code (pack bitmaps, object name disambiguation) borrows and caches the "struct multi_pack_index *" across object reads, so freeing it underneath them would be a use-after-free. Refreshing the index with proper invalidation of those borrowers is left for future work.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 odb/source-packed.c         | 29 +++++++++++++++++++++++++++
 t/t5319-multi-pack-index.sh | 40 +++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/odb/source-packed.c b/odb/source-packed.c
index 0890704e76..de96215069 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -31,6 +31,35 @@ static int find_pack_entry(struct odb_source_packed *store,
 		}
 	}
 
+	/*
+	 * Recovery for a concurrent-repack race: a MIDX can name an owning
+	 * pack for an object that a simultaneous repack has since deleted,
+	 * even though the object still exists in another pack the same MIDX
+	 * covers (e.g. a kept base pack that geometric repack did not rewrite).
+	 * If the object is present in a MIDX yet none of the paths above could
+	 * serve it, its recorded owning pack has become unavailable.  The
+	 * regular fallback above deliberately skips MIDX-covered packs, so
+	 * scan this MIDX's packs directly to find the surviving copy.  The
+	 * bsearch gate keeps genuine misses (objects absent from the MIDX) on
+	 * the fast path.
+	 */
+	if (store->midx) {
+		struct multi_pack_index *m = store->midx;
+		uint32_t midx_pos, i;
+
+		if (bsearch_midx(oid, m, &midx_pos)) {
+			for (i = 0; i < m->num_packs + m->num_packs_in_base; i++) {
+				struct packed_git *p;
+
+				if (prepare_midx_pack(m, i))
+					continue;
+				p = nth_midxed_pack(m, i);
+				if (p && packfile_fill_entry(p, oid, e))
+					return 1;
+			}
+		}
+	}
+
 	return 0;
 }
 
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 68143cb5b7..2b8ff6f3ed 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -1393,4 +1393,44 @@ test_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' '
 	)
 '
 
+test_expect_success 'lookup recovers object whose midx-owning pack was removed' '
+	test_when_finished "rm -fr repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		# "keep" ends up only in the big pack; "dup" is deliberately
+		# placed in two packs so the midx has to choose an owner.
+		test_commit keep &&
+		echo duplicated-content >dup &&
+		git add dup &&
+		git commit -m dup &&
+		dup_oid=$(git rev-parse HEAD:dup) &&
+
+		# Roll every object, including dup, into a single big pack.
+		git repack -adq &&
+
+		# Build a second, "moderate" pack that also contains dup, so dup
+		# now lives in two packs that the midx will cover.
+		moderate=$(echo "$dup_oid" |
+			git pack-objects --quiet $objdir/pack/pack) &&
+
+		# Attribute dup to the moderate pack in the midx.
+		git multi-pack-index write \
+			--preferred-pack="pack-$moderate.idx" &&
+
+		# Simulate a concurrent "git repack" retiring the moderate pack:
+		# its files disappear, but the now-stale midx still names it as
+		# the owner of dup.  A valid copy of dup survives in the big pack.
+		rm -f $objdir/pack/pack-$moderate.* &&
+
+		# The midx routes the lookup to the deleted pack, and the regular
+		# pack fallback skips midx-covered packs, so without recovery dup
+		# would appear missing even though it is physically present.
+		echo blob >expect &&
+		git cat-file -t "$dup_oid" >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done

-- 
gitgitgadget