fix(dummies): stop the Guid exclusion walk from cycling forever - #197
Merged
Conversation
AnyGuid.Generate() drew 16 random bytes and, on colliding with an excluded identifier, incremented only the last byte until it found an allowed value. When every one of the 256 last-byte variants of the drawn 15-byte prefix was excluded, that byte wrapped 255 -> 0 and the loop cycled forever -- an unbounded hang reachable with a valid, seed-derived exclusion set, contradicting the library's bounded-work contract. Propagate the carry across all 16 bytes (a new Increment helper) so the walk is the full-width successor of the drawn bytes and leaves a fully excluded prefix instead of cycling inside it -- the exclusion set can never fill the 128-bit space, so the walk terminates deterministically. This is the same escape OrdinalIntervalSpec and WideIntervalSpec already use for their 128-bit siblings. Membership is tested against a HashSet materialized once. The common no-collision path is unchanged and still consumes a single draw, so seeded sequences stay reproducible. Add a regression test that excludes all 256 last-byte variants of the drawn prefix and proves the escape terminates and stays reproducible. Refs: #192 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jktns4fipJLpBepFVqvAAb
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
2 participants
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.
Summary
AnyGuid.Generate()could loop forever when an exclusion set covered all 256 last-byte variants of the drawn 15-byte prefix: on a collision the escape incremented onlybytes[15], which wrapped255 → 0and cycled inside the excluded block. This propagates the carry across all 16 bytes so the walk leaves a fully excluded prefix and terminates — the same deterministic escape the 128-bit sibling generators (OrdinalIntervalSpec,WideIntervalSpec) already use.Type of change
Changes
AnyGuid.Generate()with a full-width 128-bit carry increment (new privateIncrementhelper), so a fully excluded 15-byte prefix can no longer trap the loop; the exclusion set can never fill the 128-bit space, so the walk terminates deterministically.HashSet<Guid>materialized once in the constructor (mirrors the existing_effectiveAllowedmaterialization), instead of rescanning the list on every step.|excluded| / 2^128.GuidExclusionByteWraparoundTerminates) that excludes all 256 last-byte variants of the drawn prefix and proves the escape terminates (off-thread deadline race, so a future regression fails the test instead of hanging the suite) and stays reproducible under a seed.The common no-collision path is unchanged and still consumes a single draw, so seeded sequences stay reproducible.
Testing
dotnet build FirstClassErrors.sln— succeeded, 0 warnings (Dummies builds for bothnetstandard2.0andnet8.0).dotnet test FirstClassErrors.sln— all projects passed (Dummies.UnitTests 154/154, and the full solution suite includingFirstClassErrors.Analyzers.UnitTestsgreen).Documentation
The public API is unchanged and the fix alters no user-facing behavior (it removes a rare hang); Guid exclusion/hang behavior is not user-documented, so no README / French translation update is needed.
Architecture decisions
The change upholds ADR-0013's bounded-work principle (a value is built without an unbounded retry loop) rather than creating, superseding, or conflicting with any decision.
Related issues
Closes #192
Generated by Claude Code