[XSG] XamlHotReloadState static cache is brittle across multiple throwaway Roslyn compilations
- Dominant language
- C#
- Stars
- 23.3k
- Forks
- 2k
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 297
Description
Follow-up to @tmat's review on #36833 (inline comment on `src/Controls/src/SourceGen/XamlHotReloadState.cs`).
### Concern
`XamlHotReloadState` holds XAML Incremental Hot Reload (XIHR) generation state in **mutable static state** and diffs the CURRENT generation against a CACHED PREVIOUS one to produce the incremental patch (the `UpdateComponent()` body).
Tomas asked (paraphrased): *"Sounds somewhat brittle. Would it work well if Roslyn created multiple compilations in the same process, ran source generators on them, and then threw them away?"*
### Why it's brittle
The cache is static (not tied to a specific `Compilation`) and survives across compilations in the same process — e.g. an IDE/design-time host, `dotnet watch`, or a test host that spins up several `Compilation`s. Interleaved or throwaway compilations can:
- **Miss a real change** — if the "previous" cached snapshot came from an unrelated/throwaway compilation, the diff against the current content can come out **empty**, so `UpdateComponent()` is emitted empty and Hot Reload silently drops a genuine XAML edit.
- **Report a spurious change** — conversely, a stale cached "previous" can make an unchanged file look changed.
This matters now that XIHR is on-by-default in Debug (#36833) and the diagnostics signal is *"empty `UpdateComponent()` body ⟺ no XAML change"*, classified pre-dispatch on `UpdateRequested.HandledTypes`. A poisoned cache corrupts that signal.
### Options
1. **Key the cache by `Compilation`** (e.g. `ConditionalWeakTable`) so throwaway compilations don't cross-contaminate and entries are collected with the compilation. Lowest-risk; keeps the empty-UC signal.
2. **Drop the cache entirely** and always full-reapply the `UpdateComponent()` body. Most robust, but then `UpdateComponent()` is *never* empty, which **breaks the empty-UC diagnostics signal** — would require moving change-classification to a **runtime `UpdateComponent()` IL diff** and re-aligning with the XamlTools team (@noiseonwires).
3. Move to incremental-generator-native caching (`IncrementalValueProvider`) instead of hand-rolled static state.
Option 1 is the likely near-term fix; option 2 is the "correct" long-term design but carries a cross-team contract implication.
### References
- Review comment: https://github.com/dotnet/maui/pull/36833#discussion_r3658710660
- Companion comment (dead `__version`/`StableContentHash`) addressed in #36912
- File: `src/Controls/src/SourceGen/XamlHotReloadState.cs`
cc @tmat
Contributor guide
Research direction
Start with src/Controls/src/SourceGen/XamlHotReloadState.cs and the UpdateComponent() body, then read the review discussion on #36833. Exercise the generator with multiple throwaway or interleaved Roslyn compilations and verify that unrelated compilations do not affect the cached diff; done means genuine XAML changes are preserved without breaking the empty UpdateComponent() diagnostics signal.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100