[Fix]Crashes in model doc page: WinUI element hierarchy and UI thread reentrancy violations #549
Conversation
weiyuanyue
changed the title
| x:Name="RootScroller" | ||
| <Grid | ||
| x:Name="ContentGrid" | ||
| Grid.Row="2" |
There was a problem hiding this comment.
not related to your change, but I feel this should be placed in Grid.Row="1"?
| try | ||
| { | ||
| Process.Start(new ProcessStartInfo() | ||
| _ = Task.Run(() => Process.Start(new ProcessStartInfo() | ||
| { | ||
| FileName = toolkitDeeplink, | ||
| UseShellExecute = true | ||
| }); | ||
| })); | ||
| } | ||
| catch | ||
| { | ||
| Process.Start(new ProcessStartInfo() | ||
| _ = Task.Run(() => Process.Start(new ProcessStartInfo() | ||
| { | ||
| FileName = "https://learn.microsoft.com/en-us/windows/ai/toolkit/", | ||
| UseShellExecute = true | ||
| }); | ||
| })); | ||
| wasDeeplinkSuccesful = false; | ||
| } |
There was a problem hiding this comment.
Task.Run is fire-and-forget so Exceptions thrown by Process.Start() inside the task will NOT be caught by the outer catch block. Consider moving the try-catch inside the Task? I know await may not satisfy your need here.
There was a problem hiding this comment.
Good point! Fixed by moving try-catch inside the Task.Run. Thank you! 👍
- Move try-catch blocks inside Task.Run to properly capture exceptions from Process.Start() - Fix Grid.Row from 2 to 1 in ModelPage.xaml (parent Grid only has 2 rows) - Ensure telemetry logging happens within Task context for accurate failure tracking
- Use DispatcherQueue.TryEnqueue to show error dialog on UI thread - Prevents crash when link opening fails in background task
There was a problem hiding this comment.
Pull request overview
This PR fixes two critical crashes in the model documentation page related to WinUI 3 framework constraints: element hierarchy violations when rendering markdown hyperlinks, and UI thread reentrancy violations when launching external processes.
Key changes:
- Added pre-validation in MyHyperlink to prevent adding InlineUIContainer elements to hyperlinks
- Moved Process.Start() calls to background threads using Task.Run() to avoid blocking the UI thread
- Removed nested ScrollViewer in ModelPage.xaml to simplify the layout hierarchy
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| AIDevGallery/Controls/Markdown/TextElements/MyHyperlink.cs | Added InlineUIContainer validation before adding children to hyperlinks; improved exception handling with debug logging |
| AIDevGallery/Pages/Models/ModelPage.xaml.cs | Wrapped Process.Start() calls in Task.Run() for AI Toolkit deeplinks and markdown link clicks; added DispatcherQueue.TryEnqueue for UI updates from background thread |
| AIDevGallery/Pages/Models/ModelPage.xaml | Removed redundant ScrollViewer wrapper around ContentGrid and cleaned up associated VisualState setter |
Fix #547 & #548 Both issues stem from fundamental misunderstandings of WinUI 3 framework constraints.
Problems
1. MyHyperlink Crash: Framework Element Hierarchy Violation
The Core Problem:
WinUI 3's
Hyperlinkclass enforces strict parent-child element relationships at the framework level. The application was attempting to addInlineUIContainerelements into hyperlinks, which WinUI explicitly prohibits. This is a fundamental architectural constraint of the WinUI framework - hyperlinks can only contain text-based inline elements, never UI containers.Why This Matters:
ArgumentExceptionwhen violations occur0x80070057(E_INVALIDARG) indicates the framework is rejecting an invalid parent-child relationshipcatchblock that silently swallowed these exceptions in some cases, masking the underlying problemRoot Cause:
The markdown rendering pipeline was blindly attempting to add any inline element to hyperlinks without validating compatibility against WinUI's element hierarchy rules. This represents a gap between the markdown DOM model (which is flexible) and WinUI's strict XAML element tree constraints.
Impact:
2. Reentrancy Violation: UI Thread Blocking in Event Handlers
The Core Problem:
WinUI 3 has a strict threading model that prohibits blocking the UI thread, especially within XAML event handlers. The application was calling
Process.Start()synchronously on the UI thread, which blocks until the external process is launched. This violates WinUI's reentrancy detection mechanism, which monitors for operations that could cause the UI message pump to become unresponsive.Why This Matters:
0x80004005,0xC000027B,0xC0000602) to prevent system-wide hangsRoot Cause:
Event handlers were written assuming synchronous operations are safe on the UI thread. However,
Process.Start()withUseShellExecute = trueinvolves shell interaction and external process launching, which can take unpredictable amounts of time. This blocked the dispatcher queue, causing WinUI to detect reentrancy and terminate the application.Impact:
Solution
Fix 1: Respect WinUI Element Hierarchy Constraints
Added pre-validation to filter out incompatible elements before attempting to add them to hyperlinks. The fix enforces WinUI's architectural rules in our code rather than relying on exception handling.
Fix 2: Honor WinUI Threading Model
Moved all
Process.Start()calls to background threads usingTask.Run(), preventing UI thread blocking and reentrancy violations.Key Changes:
Process.Start()inTask.Run()_) since we don't need to await resultsBonus: Simplified ScrollViewer Hierarchy
Removed redundant nested
ScrollViewerin Models Page that created scrolling conflicts in the layout.Result: Cleaner layout hierarchy with better scrolling behavior.
Before:
After: