[Windows]Fix for Custom TitleBar Errors During App Closure with Modal Navigation#32589
[Windows]Fix for Custom TitleBar Errors During App Closure with Modal Navigation#32589BagavathiPerumal wants to merge 1 commit intodotnet:mainfrom
Conversation
… window close when AppWindow.GetFromWindowId() returns null.
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a race condition during Windows application shutdown that causes unhandled exceptions when closing windows with modal navigation. The fix adds window state validation to prevent calling WinUI APIs on a window that's being disposed.
Key Changes
- Added early validation checks for
AppWindowIdandAppTitleBarContentControlexistence - Added null check for
AppWindow.GetFromWindowId()to detect window disposal state - Refactored code to eliminate nested conditionals by storing the
appWindowvariable
| { | ||
| var nonClientInputSrc = | ||
| InputNonClientPointerSource.GetForWindowId(AppWindowId.Value); | ||
| var nonClientInputSrc = InputNonClientPointerSource.GetForWindowId(AppWindowId.Value); |
There was a problem hiding this comment.
The PR description states that the exception "Value does not fall within the expected range" is thrown by InputNonClientPointerSource.GetForWindowId(). However, the current fix only validates that AppWindow.GetFromWindowId() returns a non-null value (line 204-206).
The call to InputNonClientPointerSource.GetForWindowId(AppWindowId.Value) on line 229 still happens after the early return checks, but there's no guarantee that this API call won't throw the same exception during window disposal. According to the PR description, this is the API that was originally failing.
Consider wrapping the InputNonClientPointerSource.GetForWindowId() call with a try-catch block or adding an additional validation check to ensure it won't throw during window closure:
InputNonClientPointerSource? nonClientInputSrc;
try
{
nonClientInputSrc = InputNonClientPointerSource.GetForWindowId(AppWindowId.Value);
}
catch (ArgumentException)
{
// Window is being disposed, exit gracefully
return;
}Alternatively, verify if the appWindow null check is sufficient to prevent this specific exception, and document why this pattern is safe.
| var nonClientInputSrc = InputNonClientPointerSource.GetForWindowId(AppWindowId.Value); | |
| InputNonClientPointerSource? nonClientInputSrc; | |
| try | |
| { | |
| nonClientInputSrc = InputNonClientPointerSource.GetForWindowId(AppWindowId.Value); | |
| } | |
| catch (ArgumentException) | |
| { | |
| // Window is being disposed, exit gracefully | |
| return; | |
| } |
|
@jfversluis hey Gerald, could we maybe have this fix in one of the next SRs? It fills up our AppCenter crashes quite a bit. Thanks! |
|
@vividos did you verify this fix actually works for you? |
|
@jfversluis Hi I'm "company vividos", author of the other PR #34032 that may fix the bug. I wasn't so sure if this PR would fix things, only work around the issue, and the AI wasn't so sure that it would fix the bug. Therefore I looked into the code and made #34032, which is also "cleaner". Thanks for looking at it, though! |
|
@MFinkBK ah! Hi! haha. OK, I triggered a build for the other PR. Can you pull those down when ready and let me know with certainty that that fixes it for you? |
|
I will do, but unfortunately I'm only available on 24th again, so @BagavathiPerumal and @sheiksyedm feel free to also test the PR's NuGet package. |
@MFinkBK, Upon further analysis, PR #34032 also appears to resolve this issue. |
|
Closing this PR in favor of #34032 |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Root Cause:
The issue occurs because of a change in the TitleBar management process that moved the cleanup call from NavigationRootManager.Disconnect() to WindowHandler.DisconnectHandler(), which unintentionally introduced a race condition during application shutdown. When the window closes, WindowHandler.DisconnectHandler() disposes of the TitleBar before NavigationRootManager.Disconnect() triggers layout updates.
These layout updates cause UpdateTitleBarContentSize() to run, which attempts to access WinUI APIs InputNonClientPointerSource.GetForWindowId(). Because the window is in the process of closing, these APIs can fail and raise exception “Value does not fall within the expected range.” The absence of proper window state validation before these API calls resulted in unhandled exceptions during shutdown.
Regression PR: #27192
Fix Description:
The fix involves adding window state validation at the start of UpdateTitleBarContentSize() before calling any window-specific APIs. The method now verifies that AppWindowId is valid, AppTitleBarContentControl exists, and AppWindow.GetFromWindowId()does not return null. If any of these checks fail, the method exits gracefully. This prevents exceptions during shutdown and allows the cleanup process to complete smoothly without extra state tracking.
When the window is being disposed, AppWindow.GetFromWindowId() returns null, causing the method to terminate safely before invoking APIs like InputNonClientPointerSource.GetForWindowId(), thereby avoiding exceptions and eliminating the need for additional error handling.
Note: This issue occurs when closing the window, so it is not possible to add a test case for it.
Issues Fixed
Fixes #32331
Fixes #32194
Tested the behaviour in the following platforms
Currently, the TitleBar control is only available on the Windows and Mac Catalyst.
Output Screenshot
Beforefix.32331.mp4
Afterfix.32331.mp4