Skip to content

[✨ Triage] dotnet/runtime#124659 by MihaZupan - Harmless debug assert hit in Uri's DebugAssertInCtor #132

@MihuBot

Description

@MihuBot

Triage for dotnet/runtime#124659.
Repo filter: All networking issues.
MihuBot version: 3d5c38.
Ping MihaZupan for any issues.

This is a test triage report generated by AI, aimed at helping the triage team quickly identify past issues/PRs that may be related.
Take any conclusions with a large grain of salt.

Tool logs
dotnet/runtime#124659: Harmless debug assert hit in Uri's DebugAssertInCtor by MihaZupan
Extracted 5 search queries: Debug.Assert (_flags & Flags.Debug_LeftConstructor) == 0 in System.Uri.CreateThisFromUri, Uri re-entrant construction combining UNC file base with a relative "file:" URI causes assert, Debug_LeftConstructor flag copied or preserved in CreateThisFromUri leading to DebugAssertInCtor, DebugSetLeftCtor / DebugAssertInCtor invariant violation during Uri constructor re-entry, new Uri(baseUri, relUri) where base is UNC and rel is 'file:' triggers Debug.Assert in Uri
Found 25 candidate issues
  • Pull Request #123932 (Feb 2026) - Fix parsing inconsistencies between Uri ctors and TryCreate factories
    Summary: This refactor unified ctor and TryCreate code paths (TryCreateThis) so both apply the same IRI normalization and fallback-to-relative logic. During review a few observations were raised that are directly relevant to your new issue:

    • The refactor introduced a private CreateHelper / CreateThis flow that allocates and returns a Uri instance in some code paths without calling DebugSetLeftCtor(). Public TryCreate wrappers set the left-ctor debug state, but internal callers that call CreateHelper directly may receive a Uri that still appears to be "in-construction" in DEBUG builds.
    • A code-review comment (copilot-generated) suggested calling result.DebugSetLeftCtor() before returning a successfully created Uri from CreateHelper to avoid leaving the Debug_LeftConstructor flag set on instances returned to callers.
    • The PR was merged; the new issue you opened references the extra DebugAssertInCtor added as part of DebugSetLeftCtor in this change. This PR is the most likely source of the introduced DebugAssert invariant failure (CreateThisFromUri copies flags including Debug_LeftConstructor, so an instance returned from the re-entrant construction path can violate the assert). Conclusion: #123932 unified parsing behavior but reviewers flagged (and documented) the exact kind of debug-state mis-handling that can produce the assert you hit. The fix direction called out in review is to ensure DebugSetLeftCtor is applied consistently (or to avoid copying the Debug_LeftConstructor flag when merging/copying flags).
  • Issue #124659 (Feb 2026) - Harmless debug assert hit in Uri's DebugAssertInCtor (the new issue)
    Summary: Reproducer: new Uri(new Uri(@"\aa", UriKind.Absolute), new Uri("file:", UriKind.Relative)) triggers Debug.Assert: (_flags & Flags.Debug_LeftConstructor) == 0 inside DebugAssertInCtor. The report notes this is caused by CreateThisFromUri copying the Debug_LeftConstructor flag during a re-entrant construction from a UNC base + file: relative combo. Author suggests either removing the assert or avoiding copying the Debug_LeftConstructor flag in CreateThisFromUri. (This is the issue you are triaging.)

  • Pull Request #124660 (Feb 2026) - [WIP] Fix debug assert in Uri's constructor handling
    Summary: An automatically-created WIP PR (copilot) targeting this exact issue. It currently has no changes (0 files changed); it appears to be a placeholder/working branch. No discussion or code yet. Conclusion: an attempt has been started but nothing landed; the active, reviewed fix likely needs to be applied manually (see #123932 review notes about DebugSetLeftCtor).

  • Pull Request #123054 (Jan 2026) - Fix edge-case Uri debug assert failure
    Summary: Example of a prior "harmless" debug-assert in Uri internals where path compression could produce a ValueStringBuilder access that triggered an assert but was harmless in practice. The PR fixed the assert by adding an appropriate bounds check and included tests. Conclusion/precedent: when debug-only asserts are hit but the behavior is harmless, the right fix is often to make the internal checks robust (add guards) rather than removing the assert wholesale.

  • Issue #119547 (Sep 2025) - System.Uri.AssertInvariants failing with debug assert: offset.Query == 0
    Summary: A CI hit showed AssertInvariants failing because code assumed Query/Fragment offsets were not populated unless AllUriInfoSet was set. Miha explained those asserts have a race condition (a different thread may parse between checking a flag and reading the offset). That assertion was introduced earlier and the race made it noisy in CI. Conclusion: debug assertions in Uri can be racy or fragile when flags/parse state can be mutated by other threads (a precedent for cautious handling / either avoiding certain asserts or making the code robust to concurrent parse progress).

  • Pull Request #121653 (Nov 2025) - Remove unsafe code in CreateUriInfo and fix edge-case IndexOutOfRangeExceptions
    Summary: Fixed parsing and offset-tracking bugs when Uri strings are rebuilt for non-ASCII/IRI handling; removed unsafe pointer code and corrected mistaken indexing between original and rebuilt strings. Conclusion: changes in parsing internals and offsets happen frequently and have subtle effects; fixing internal invariants there (instead of removing asserts) was the approach taken.

  • Issue #85229 (Apr 2023) - Inconsistency when combining file: URIs
    Summary: Longstanding behavior note: Uri treats "explicit file:" inputs (strings that include "file://") as true URIs (with fragment support), but treats implicit file-path inputs as plain paths (no fragment). Miha explained this is by-design and recommended using explicit file: URIs when you need URI semantics. Relevance: your repro involves a UNC base parsed as file://aa/ combined with a "file:" relative — the area is known to be tricky and behavior differences between implicit/explicit file handling have been the source of many edge cases. Conclusion: file/UNC handling is delicate; the root assert in your case sits at the intersection of parsing/merging logic and the debug-state handling introduced in recent refactors.

Other notes and takeaways from the related work

  • The most actionable lead from reviewed PR discussion is the Code Review comment on #123932: ensure CreateHelper (or any internal helper that returns a constructed Uri) sets the debug "left constructor" state consistently (i.e., call DebugSetLeftCtor() before returning), or make CreateThisFromUri avoid copying the Debug_LeftConstructor flag when materializing from another Uri. Either approach resolves the invariant the new assert enforces.
  • There is an established pattern in the repo for addressing "harmless" debug asserts either by (a) making internal checks more robust (add guards) or (b) ensuring the debug-state lifecycle is correct so the assertion is never violated. The prior fixes (#123054, #121653) favored correcting the logic/guards rather than removing assertions.
  • Be mindful of race conditions with parse-state flags (see #119547) — any fix that relies on checking flags across threads or during re-entrant parsing should avoid introducing new races or brittle assertions.

If you want, I can:

  • point to the exact locations in the merged #123932 where the DebugSetLeftCtor/logical gap exists (CreateHelper / CreateThisFromUri) so you can draft a small change (either set DebugSetLeftCtor before returning, or mask Debug_LeftConstructor when copying flags), or
  • draft a minimal PR that masks the Debug_LeftConstructor bit in CreateThisFromUri when copying flags (likely the minimal change to preserve the new debug assertion invariant).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions