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

On Thu, Aug 20, 2026 at 12:54 AM Patrick Steinhardt <[email protected]> wrote:
>
> On Tue, Aug 18, 2026 at 10:34:06PM +0000, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <[email protected]>
> >
> > 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.
>
> Okay. Rephrasing in my own words: the object in question exists in two
> packs covered by the MIDX. We rewrite one of those two packs, and the
> MIDX used to reference the object via the pack we're about to rewrite.
> Consequently, the MIDX is stale now and it cannot be used to find the
> object anymore because its pack has disappeared. And as we know to skip
> searching packfiles for the object that are already covered by the MIDX
> we won't be able to find it via the second packfile, either.

Yep.

> > 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.
>
> Hm. Isn't there a slight variant of the race though for any caller that
> does not use OBJECT_INFO_QUICK?
>
> Namely, the packfile containing our object disappears and is being
> written to a new packfile, and that file is the only one containing it.
> Without OBJECT_INFO_QUICK we would be fine: we notice the object could
> not be found, and then we perform a second read that makes the "packed"
> backend reload its packfiles. It would find the new packfile, and
> because it's not covered by its MIDX it would use it to surface the
> object. But without OBJECT_INFO_QUICK that's not the case, as we would
> skip reloading packfiles altogether, and hence we would not be able to
> find that object at all.
>
> As far as I can see though, we don't seem to pass OBJECT_INFO_QUICK in
> any of the mentioned readers. I could very well be missing something
> here, but I would have thought that those readers are fine in this
> scenario?

Nicely caught -- and you're right that the readers named above are
fine: they're all non-QUICK, so the second read reloads the packfiles
and finds the object in its new, non-MIDX-covered home, exactly as you
describe.

But the variant you describe is a real bug for QUICK callers that
don't get that second read -- e.g. upload-pack's object-existence
checks and mktree --batch.  I have three more race-condition patches
to clean up and submit, and this is one of them: it forces the reload
even under OBJECT_INFO_QUICK once we notice a pack has vanished out
from under us.

Your wording also makes me realize that my fix in this unsubmitted
patch still has a hole: it triggers when opening the pack .idx fails,
but if the timing is such that the .idx is already mmapped and only
the .pack has gone missing, it won't fire.  I'll look into that before
submitting...and then clean up/submit my two other race fixes as well.