Re: [PATCH] fsck: do not loop infinitely when processing packs

[Jeff King <peff@peff.net>]

On Sun, Feb 22, 2026 at 11:07:41PM +0000, brian m. carlson wrote:

> I noticed that the code here seems to have come in with the 2.53 cycle,
> so we may want to cherry-pick it to `maint` at some point if it seems
> like the problem occurs often.  From what I can tell, it only occurs
> when one explicitly invokes `git fsck`[0] and not on transfer, so it
> shouldn't cause a DoS against server implementations.
> 
> Of course, we should wait for Patrick, who authored this code, to chime
> in and lend his expertise here.  I must admit I'm not very familiar with
> this area, although I had recently seen the MRU code when working on
> pack index v3 (and then I thought, "is this actually the problem?").

The problem seems to bisect to c31bad4f7d (packfile: track packs via the
MRU list exclusively, 2025-10-30), which is not terribly surprising, as
it was one of the known risks of collapsing the two lists into one.

Your solution is using the tool provided by that commit for its edge
case:

    Note that there is one important edge case: `for_each_packed_object()`
    uses the MRU list to iterate through packs, and then it lists each
    object in those packs. This would have the effect that we now sort the
    current pack towards the front, thus modifying the list of packfiles we
    are iterating over, with the consequence that we'll see an infinite
    loop. This edge case is worked around by introducing a new field that
    allows us to skip updating the MRU.

So in that sense it is the right thing. But it really makes me wonder if
we are going back to keeping two lists (one MRU and one in some stable
order). Or at the very least providing _some_ iteration method that is
guaranteed to be stable (whether a linked list or a function), so that
iterating code is not subject to this subtle dependency by default.

Having to identify each potential spot and set a "btw, don't switch the
pack list order!" flag seems error-prone. And also loses efficiency when
you are iterating a pack and accessing objects in it (since we can't
push that pack to the front of the MRU then, even though we'd expect
there to be high locality with our iteration).

-Peff