Fix infinite loop on IsValidBase - #20
Merged
hahn-kev merged 3 commits intoAug 4, 2026
Merged
Conversation
On an empty repo (hg init, zero changesets), GetRevisionsInternal hit its empty-repo special case and returned the "0:" sentinel for every offset, because the check ran before Skip(offset).Take(quantity). IsValidBase then paged forever: any baseHash other than "0" never matched, the page was never empty, and the offset grew without bound, hanging the pull request. Fix the root cause (only emit the sentinel at offset 0; return empty past it) and add a defensive guard in IsValidBase: a page shorter than the requested quantity is the last page, so stop rather than advancing the offset again. This guarantees the loop terminates even against any future GetRevisions quirk that returns a fixed non-empty page regardless of offset. Add an HTTP-level regression test that pulls an empty repo with a non-zero baseHash and asserts it returns FAIL within a timeout instead of hanging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…de/csharp-isvalidbase-empty-repo
Add a >200-revision fixture (manyRevsHgRepo, 205 commits) and a pull test that requests a non-existent baseHash against it. This forces IsValidBase past its first full page (offset 0 -> 200) onto a short final page, exercising the offset-advancement and short-page-break branch that the single-page InvalidHash test never reaches. Asserts FAIL within a timeout so a non-terminating loop would surface as a fast failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this caused a crash in production recently, we're fixing it.
🤖 AI summary
On an empty repo (
hg init, zero changesets),IsValidBasehung the pull request in an infinite loop.GetRevisionsInternalhandles an empty repo with a special case that returns the"0:"sentinel — but the check ran beforeSkip(offset).Take(quantity), so it returned that same sentinel for every offset.IsValidBasethen paged forever: anybaseHashother than"0"never matched, the page was never empty, and the offset grew without bound.Fix — two complementary parts:
GetRevisionsInternal): only emit the"0:"sentinel at offset 0; paginating past it returns an empty list, so callers see a terminating page. The offset-0 contract (getRevisions,GetTip,GetBranchTips) is unchanged.IsValidBase): a page shorter than the requested quantity is the last page, so stop rather than advancing the offset again. This guarantees termination even against any futureGetRevisionsquirk that returns a fixed non-empty page regardless of offset.Either part alone stops the hang; together they fix the cause and make the loop structurally incapable of running forever.
Also adds an HTTP-level regression test (
PullBundleChunk_EmptyRepoWithNonZeroBaseHash_FailsWithoutHanging) that pulls an empty repo with a non-zerobaseHashand assertsFAILwithin a timeout instead of hanging.Test plan
Verified against the rebuilt image (repos seeded before container start, to sidestep a local Docker-Desktop
docker cp-into-running-container visibility quirk):baseHash=0NOCHANGENOCHANGEbaseHash=fakehashFAIL(~0.5s)NOCHANGENOCHANGEFAILFAILgetRevisionsoffset 0 on empty repo0:sentinel0:sentinel (unchanged)The new xUnit test runs in CI (Linux); it can't seed repos through the fixture locally on Windows due to the
docker cpoverlay quirk noted above, which also affects the pre-existing empty-repo test.This change is