Fix crash when displaying alerts on unloaded pages#33288
Open
kubaflo wants to merge 3 commits intodotnet:mainfrom
Open
Fix crash when displaying alerts on unloaded pages#33288kubaflo wants to merge 3 commits intodotnet:mainfrom
kubaflo wants to merge 3 commits intodotnet:mainfrom
Conversation
Contributor
Author
This was referenced Dec 25, 2025
pictos
reviewed
Dec 25, 2025
src/Controls/src/Core/Page/Page.cs
Outdated
| if (Window is null) | ||
| { | ||
| // Complete the task with cancel result | ||
| args.SetResult(cancel); |
Contributor
There was a problem hiding this comment.
Could we add a log here? As a maui developer would be great to know why the Alert didn't show
Contributor
Author
There was a problem hiding this comment.
Good call! Something like this?
if (Window is null)
{
args.SetResult("DisplayActionSheetAsync: Window is null, action sheet will not be shown. This can happen if the page is not attached to a window.");
return args.Result.Task;
}
Contributor
There was a problem hiding this comment.
Not sure how that will be displayed. But yeah, the message looks good. I was thinking in something like Trace.WriteLine to be shown into the logs/output window
This was referenced Dec 26, 2025
Open
PureWeen
requested changes
Dec 26, 2025
Member
PureWeen
left a comment
PureWeen
left a comment
There was a problem hiding this comment.
From Copilot
UITest Validation Issue
Great fix - the Page.cs changes look solid! However, I found an issue with the UITest.
**Problem**: I reverted your Page.cs fix and ran the test twice - it still passed both times even though the app crashed (device logs showed "app died, no saved state").
**Root cause**: The test asserts `status.Does.Not.Contain("NullReferenceException")`, but the fatal crash kills the app before the exception can be written to the
StatusLabel. The test reads the last status before the crash ("Showing alert from unloaded page...") which doesn't contain that text.
**Suggested fix**: Check for positive success instead of absence of error:
```csharp
// Current (doesn't catch regression):
Assert.That(status, Does.Not.Contain("NullReferenceException"), ...);
// Suggested (will fail without fix):
Assert.That(status, Does.Contain("✅"),
"App should show success status after DisplayAlertAsync completes");
To verify yourself:
- Revert src/Controls/src/Core/Page/Page.cs to before your fix
- Run: pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform android -TestFilter "Issue33287"
- Test should fail but currently passes
kubaflo
added
s/agent-changes-requested
kubaflo
changed the title
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 33288Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 33288" |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a crash in Microsoft.Maui.Controls.Page when alert-related APIs are invoked after a page has been navigated away/unloaded (i.e., Window becomes null), and adds a UI test reproduction to prevent regressions.
Changes:
- Add
Window == nullhandling forDisplayActionSheetAsync,DisplayAlertAsync, andDisplayPromptAsyncinPage.cs. - Add a HostApp issue page to reproduce the unload + delayed dialog scenario.
- Add an Appium/NUnit UI test validating the app doesn’t crash and no
NullReferenceExceptionis surfaced.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/Page/Page.cs | Adds Window null handling and result completion defaults for alert/action sheet/prompt APIs. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs | Adds a navigation-based repro page which triggers a delayed DisplayAlertAsync after navigating away. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs | Adds an Appium UI test to exercise the scenario and detect crashes/exceptions. |
src/Controls/src/Core/Page/Page.cs
Outdated
Comment on lines
+450
to
+460
| if (IsPlatformEnabled) | ||
| Window.AlertManager.RequestPrompt(this, args); | ||
| else | ||
| _pendingActions.Add(() => Window.AlertManager.RequestPrompt(this, args)); | ||
| _pendingActions.Add(() => | ||
| { | ||
| // Check again in case window was detached while waiting | ||
| if (Window is not null) | ||
| Window.AlertManager.RequestPrompt(this, args); | ||
| else | ||
| args.SetResult(null); | ||
| }); |
Comment on lines
+7
to
+14
| public partial class Issue33287 : NavigationPage | ||
| { | ||
| public Issue33287() : base(new Issue33287MainPage()) | ||
| { | ||
| } | ||
| } | ||
|
|
||
| public partial class Issue33287MainPage : ContentPage |
src/Controls/src/Core/Page/Page.cs
Outdated
Comment on lines
+327
to
+333
| // If page is no longer attached to a window (e.g., navigated away), ignore the action sheet request | ||
| if (Window is null) | ||
| { | ||
| Trace.WriteLine("DisplayActionSheetAsync: Window is null, action sheet will not be shown. This can happen if the page is not attached to a window."); | ||
| args.SetResult(cancel); | ||
| return args.Result.Task; | ||
| } |
src/Controls/src/Core/Page/Page.cs
Outdated
Comment on lines
+335
to
+343
| if (IsPlatformEnabled) | ||
| Window.AlertManager.RequestActionSheet(this, args); | ||
| else | ||
| _pendingActions.Add(() => Window.AlertManager.RequestActionSheet(this, args)); | ||
| _pendingActions.Add(() => | ||
| { | ||
| // Check again in case window was detached while waiting | ||
| if (Window is not null) | ||
| Window.AlertManager.RequestActionSheet(this, args); | ||
| else |
src/Controls/src/Core/Page/Page.cs
Outdated
Comment on lines
+411
to
+421
| if (IsPlatformEnabled) | ||
| Window.AlertManager.RequestAlert(this, args); | ||
| else | ||
| _pendingActions.Add(() => Window.AlertManager.RequestAlert(this, args)); | ||
| _pendingActions.Add(() => | ||
| { | ||
| // Check again in case window was detached while waiting | ||
| if (Window is not null) | ||
| Window.AlertManager.RequestAlert(this, args); | ||
| else | ||
| args.SetResult(false); | ||
| }); |
src/Controls/src/Core/Page/Page.cs
Outdated
Comment on lines
+442
to
+448
| // If page is no longer attached to a window (e.g., navigated away), ignore the prompt request | ||
| if (Window is null) | ||
| { | ||
| // Complete the task with null result | ||
| args.SetResult(null); | ||
| return args.Result.Task; | ||
| } |
Comment on lines
+31
to
+33
| // Wait for the delayed DisplayAlertAsync to be triggered (5 seconds + buffer) | ||
| System.Threading.Thread.Sleep(6000); | ||
|
|
src/Controls/src/Core/Page/Page.cs
Outdated
| // If page is no longer attached to a window (e.g., navigated away), ignore the action sheet request | ||
| if (Window is null) | ||
| { | ||
| Trace.WriteLine("DisplayActionSheetAsync: Window is null, action sheet will not be shown. This can happen if the page is not attached to a window."); |
src/Controls/src/Core/Page/Page.cs
Outdated
Comment on lines
+403
to
+409
| // If page is no longer attached to a window (e.g., navigated away), ignore the alert request | ||
| if (Window is null) | ||
| { | ||
| // Complete the task with default result (cancel) | ||
| args.SetResult(false); | ||
| return args.Result.Task; | ||
| } |
Contributor
Author
|
/rebase |
github-actions
bot
force-pushed
the
fix-33287
branch
from
8a80beb to
43f5750
Compare
Added null checks for the Window property in DisplayAlertAsync, DisplayActionSheetAsync, and DisplayPromptAsync methods in Page.cs to prevent NullReferenceException when the page is no longer attached to a window. New UI tests verify that async alert requests do not crash the app after navigating away from the page.
Added a Trace.WriteLine statement to log when DisplayActionSheetAsync is called but the page is not attached to a window. This helps with debugging scenarios where the action sheet is not shown due to the page being detached.
…e tests - Capture Window in local variable to fix TOCTOU race (Copilot review) - Only early-return when IsPlatformEnabled && Window is null to preserve pending action queuing for not-yet-attached pages (Copilot review) - Add consistent Trace.WriteLine logging across all three methods (pictos) - Assert positive success (contains) instead of absence of error (PureWeen) - Replace Thread.Sleep with polling loop for deterministic wait (Copilot review) - Rename .xaml.cs to .cs and remove partial keywords (no XAML file) (Copilot review) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
🚦 Gate - Test Before and After Fix📊 Expand Full Gate —
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🖥️ Issue33287 Issue33287 |
✅ FAIL — 141s | ❌ FAIL — 62s |
🔴 Without fix — 🖥️ Issue33287: FAIL ✅ · 141s
Determining projects to restore...
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/BindingSourceGen/Controls.BindingSourceGen.csproj (in 432 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 548 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 8.41 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/Foldable/src/Controls.Foldable.csproj (in 8.5 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 8.5 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj (in 8.51 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 8.51 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 8.51 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/BlazorWebView/src/Maui/Microsoft.AspNetCore.Components.WebView.Maui.csproj (in 8.51 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 8.52 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/maps/src/Maps.csproj (in 8.52 sec).
/Users/cloudtest/vss/_work/1/s/.dotnet/packs/Microsoft.iOS.Sdk.net10.0_26.0/26.0.11017/targets/Xamarin.Shared.Sdk.targets(309,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-ios]
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0-ios26.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0-ios26.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0-ios26.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0-ios26.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Microsoft.AspNetCore.Components.WebView.Maui -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-ios26.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Foldable -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Foldable.dll
Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Maps.dll
Detected signing identity:
Code Signing Key: "" (-)
Provisioning Profile: "" () - no entitlements
Bundle Id: com.microsoft.maui.uitests
App Id: com.microsoft.maui.uitests
Controls.TestCases.HostApp -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-ios/iossimulator-arm64/Controls.TestCases.HostApp.dll
Optimizing assemblies for size may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
Optimizing assemblies for size. This process might take a while.
Build succeeded.
/Users/cloudtest/vss/_work/1/s/.dotnet/packs/Microsoft.iOS.Sdk.net10.0_26.0/26.0.11017/targets/Xamarin.Shared.Sdk.targets(309,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-ios]
1 Warning(s)
0 Error(s)
Time Elapsed 00:01:38.08
Determining projects to restore...
Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/VisualTestUtils/VisualTestUtils.csproj (in 601 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/BindingSourceGen/Controls.BindingSourceGen.csproj (in 601 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 601 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/CustomAttributes/Controls.CustomAttributes.csproj (in 601 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 601 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/UITest.Core/UITest.Core.csproj (in 601 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 658 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 671 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/UITest.NUnit/UITest.NUnit.csproj (in 1.45 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/UITest.Appium/UITest.Appium.csproj (in 1.65 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/UITest.Analyzers/UITest.Analyzers.csproj (in 1.82 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj (in 4.52 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.iOS.Tests/Controls.TestCases.iOS.Tests.csproj (in 5.18 sec).
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Controls.CustomAttributes -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
UITest.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
VisualTestUtils -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
UITest.NUnit -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
VisualTestUtils.MagickNet -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
UITest.Appium -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
UITest.Analyzers -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs(35,47): error CA1307: 'string.Contains(string)' has a method overload that takes a 'StringComparison' parameter. Replace this call in 'Microsoft.Maui.TestCases.Tests.Issues.Issue33287.DisplayAlertAsyncShouldNotCrashWhenPageUnloaded()' with a call to 'string.Contains(string, System.StringComparison)' for clarity of intent. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1307) [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.iOS.Tests/Controls.TestCases.iOS.Tests.csproj]
🟢 With fix — 🖥️ Issue33287: FAIL ❌ · 62s
Determining projects to restore...
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/BindingSourceGen/Controls.BindingSourceGen.csproj (in 314 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 322 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 325 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 366 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 378 ms).
6 of 11 projects are up-to-date for restore.
/Users/cloudtest/vss/_work/1/s/.dotnet/packs/Microsoft.iOS.Sdk.net10.0_26.0/26.0.11017/targets/Xamarin.Shared.Sdk.targets(309,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-ios]
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0-ios26.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0-ios26.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0-ios26.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0-ios26.0/Microsoft.Maui.Maps.dll
Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Microsoft.AspNetCore.Components.WebView.Maui -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-ios26.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Foldable -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Foldable.dll
Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Maps.dll
Detected signing identity:
Code Signing Key: "" (-)
Provisioning Profile: "" () - no entitlements
Bundle Id: com.microsoft.maui.uitests
App Id: com.microsoft.maui.uitests
Controls.TestCases.HostApp -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-ios/iossimulator-arm64/Controls.TestCases.HostApp.dll
Optimizing assemblies for size may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
Optimizing assemblies for size. This process might take a while.
Build succeeded.
/Users/cloudtest/vss/_work/1/s/.dotnet/packs/Microsoft.iOS.Sdk.net10.0_26.0/26.0.11017/targets/Xamarin.Shared.Sdk.targets(309,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-ios]
1 Warning(s)
0 Error(s)
Time Elapsed 00:00:38.52
Determining projects to restore...
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 759 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/BindingSourceGen/Controls.BindingSourceGen.csproj (in 753 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 760 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 783 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 801 ms).
8 of 13 projects are up-to-date for restore.
Controls.CustomAttributes -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.60-ci+azdo.13724769
Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
VisualTestUtils -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
UITest.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
VisualTestUtils.MagickNet -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
UITest.NUnit -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
UITest.Appium -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
UITest.Analyzers -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs(35,47): error CA1307: 'string.Contains(string)' has a method overload that takes a 'StringComparison' parameter. Replace this call in 'Microsoft.Maui.TestCases.Tests.Issues.Issue33287.DisplayAlertAsyncShouldNotCrashWhenPageUnloaded()' with a call to 'string.Contains(string, System.StringComparison)' for clarity of intent. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1307) [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.iOS.Tests/Controls.TestCases.iOS.Tests.csproj]
⚠️ Issues found
- ❌ Issue33287 FAILED with fix (should pass)
📁 Fix files reverted (8 files)
eng/pipelines/ci-copilot.ymlsrc/Controls/src/Core/FlyoutPage/FlyoutPage.cssrc/Controls/src/Core/Page/Page.cssrc/Controls/src/SourceGen/CSharpExpressionHelpers.cssrc/Controls/src/SourceGen/KnownMarkups.cssrc/Controls/src/SourceGen/SetPropertyHelpers.cssrc/Core/src/Handlers/FlyoutView/FlyoutViewHandler.Android.cssrc/Core/src/Platform/Android/Navigation/NavigationViewFragment.cs
New files (not reverted):
github-merge-flow-release-11.jsonc
Collaborator
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #33288 | Add Window is null null checks inside IsPlatformEnabled branch in all 3 display methods + pending actions null check |
❌ GATE FAILED (CA1307 compile error in test, not in fix) | Page.cs |
Fix logic is correct; test needs CA1307 fix |
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (opus-4.6) | Proactive pending-action cancellation via OnPropertyChanged + typed tuple _pendingActions with Dispatch/Fallback + centralized TryDispatchToAlertManager helper |
✅ Pass | Page.cs, Issue33287.cs (CA1307 fix) |
Architecturally different from PR; event-driven cleanup |
| 2 | try-fix (sonnet-4.6) | IsLoaded semantic guard before IsPlatformEnabled branch in all 3 methods |
✅ Pass | Page.cs, Issue33287.cs (CA1307 fix) |
Simpler than PR; also fixes hang scenario when IsPlatformEnabled=false and page permanently popped |
| 3 | try-fix (gpt-5.3-codex) | Window-captured dispatch: capture window = Window once; if null, return immediate fallback (no queue). Guards both NRE and hang paths. |
✅ Pass | Page.cs, Issue33287.cs (CA1307 fix) |
Similar to PR but adds else branch guard too; simpler than Attempt 1 |
| 4 | try-fix (gpt-5.4) | Cancel queued dialog pending actions in OnHandlerChanging when handler is disconnected — root-cause lifecycle fix |
✅ Pass | Page.cs, Issue33287.cs (CA1307 fix) |
Architecturally addresses root cause; fixes hang path |
| PR | PR #33288 | Inline window is null checks inside IsPlatformEnabled=true branch; pending action lambda also checks Window | ❌ Gate FAILED (CA1307 in test) | Page.cs | Fix logic partially correct; misses hang when IsPlatformEnabled=false + permanently popped |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | "NO NEW IDEAS — 4 attempts cover pre-check guards and reactive cancellation families comprehensively" |
Exhausted: Yes
Selected Fix: Candidate #2 (Attempt 2, IsLoaded guard) — Simplest solution (9 lines across 3 methods), catches both NRE and hang scenarios, uses existing IsLoaded property already present in Page.cs, higher-level semantic check vs raw Window is null.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #33287, 3 files changed, CA1307 test error identified |
| Gate | ❌ FAILED | iOS — test compiled to error CA1307 (both with and without fix) |
| Try-Fix | ✅ COMPLETE | 4 attempts, 4 passing; best fix = Attempt 2 (IsLoaded guard) |
| Report | ✅ COMPLETE |
Summary
PR #33288 correctly identifies the root cause (null Window when DisplayAlertAsync/DisplayActionSheetAsync/DisplayPromptAsync are called on a page removed from the navigation stack) and the core fix logic in Page.cs is sound. However, the Gate failed because the new test file has a CA1307 compile error (string.Contains("✅") without StringComparison), preventing the test from building at all.
Additionally, try-fix exploration revealed a hang scenario the PR misses: when IsPlatformEnabled=false and the page is permanently popped, the pending action is queued but NavigatedTo never fires, so the Task hangs forever. The IsLoaded-guard approach (Attempt 2) fixes both the NRE crash and the hang with a simpler, more semantic check.
Root Cause
When a page is popped from the navigation stack, its Window property becomes null. The three display methods accessed Window.AlertManager without null-checking. Additionally, if the handler was already disconnected (IsPlatformEnabled=false), calling these methods queued an action in _pendingActions which is never flushed for a permanently-popped page, causing the awaiting Task to hang.
Fix Quality
Issues with PR's fix:
-
CA1307 compile error in test (critical — blocks all CI validation):
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs:35—status.Contains("✅")must useStringComparisonoverload:status.Contains("✅", StringComparison.Ordinal)
-
Hang scenario not addressed (logic gap):
- When
IsPlatformEnabled=falseand the page is permanently popped, the pending action is queued butNavigatedTo(which flushes_pendingActions) never fires. The awaiting Task hangs forever. The PR's null check is only inside theIsPlatformEnabled=truebranch.
- When
-
Test uses
Thread.Sleeppolling loop (anti-pattern):- Lines 35–40 use
System.Threading.Thread.Sleep(1000)in a retry loop. This should useApp.WaitForElementwith a timeout (per UI test guidelines).
- Lines 35–40 use
-
Unnecessary
partialkeyword in HostApp (minor):Issue33287MainPageinIssue33287.csis declaredpartialbut there is no XAML counterpart.
Better fix — Attempt 2 (IsLoaded guard):
Add if (!IsLoaded) { args.SetResult(<default>); return args.Result.Task; } before the IsPlatformEnabled branch in all three methods. This is:
- Simpler: ~9 lines across 3 methods vs. 6 inline checks in the PR
- More complete: Catches both NRE (Window=null, IsPlatformEnabled=true) AND hang (IsPlatformEnabled=false, permanently popped)
- More semantic:
IsLoadedis a higher-level, well-understood property already used inPage.cs - Consistent: Same guard at the same location in all 3 methods
Selected Fix: Candidate #2 (Attempt 2) — IsLoaded guard before IsPlatformEnabled branch
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
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.
Fix for Issue #33287 - DisplayAlertAsync NullReferenceException
Issue Summary
Reporter: @mfeingol
Platforms Affected: All (Android reported, likely all)
Version: 10.0.20
Problem: Calling
DisplayAlertAsync(orDisplayActionSheetAsync,DisplayPromptAsync) on a page that has been navigated away from results in aNullReferenceException, crashing the app.Reproduction Scenario:
OnAppearing()DisplayAlertAsyncWindowproperty is nullRoot Cause
Location:
src/Controls/src/Core/Page/Page.cslines 388, 390When a page is unloaded (removed from navigation stack), its
Windowproperty becomesnull. TheDisplayAlertAsync,DisplayActionSheetAsync, andDisplayPromptAsyncmethods accessedWindow.AlertManagerwithout null checking:Stack Trace (from issue report):
Solution
Added null checks for
Windowproperty in three methods. WhenWindowis null (page unloaded), complete the task gracefully with sensible defaults instead of crashing.Files Modified
src/Controls/src/Core/Page/Page.csDisplayAlertAsync (lines 376-407)
Window.AlertManagerfalse(cancel) when window is nullDisplayActionSheetAsync (lines 321-347)
Window.AlertManagercancelbutton text when window is nullDisplayPromptAsync (lines 422-463)
Window.AlertManagernullwhen window is nullImplementation
Why this approach:
Testing
Reproduction Test Created
Files:
src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs- Test page with navigationsrc/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs- NUnit UI testTest Scenario:
DisplayAlertAsyncafter 5-second delayTest Results
Before Fix:
After Fix:
Platform Tested: Android API 36 (Pixel 9 emulator)
Edge Cases Verified
Behavior Changes
Before Fix
NullReferenceExceptionAfter Fix
DisplayAlertAsync→false(cancel)DisplayActionSheetAsync→ cancel button textDisplayPromptAsync→nullRationale: If user navigated away, they didn't see the alert, so returning "cancel" is semantically correct.
Breaking Changes
None. This is purely a bug fix that prevents crashes.
Impact:
Additional Notes
Why This Wasn't Caught Earlier
This is a timing/race condition issue:
Workaround (Before Fix)
Users had to manually check
IsLoadedproperty:With this fix, this workaround is no longer necessary.
Files Changed Summary
Related Issues
WindowpropertyWindow.accesses in Page class for similar issuesPR Checklist