-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix SwipeViews with invoked properties crash the app in Release mode #35208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| <?xml version="1.0" encoding="utf-8" ?> | ||
| <ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui" | ||
| xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" | ||
| xmlns:local="clr-namespace:Microsoft.Maui.Controls.Xaml.UnitTests" | ||
| x:Class="Microsoft.Maui.Controls.Xaml.UnitTests.Maui18055"> | ||
| <ContentPage.Resources> | ||
| <DataTemplate x:Key="virtualHandlerTemplate"> | ||
| <local:ElementWithEvent Clicked="HandleVirtualClicked" /> | ||
| </DataTemplate> | ||
| </ContentPage.Resources> | ||
| </ContentPage> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| using System; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Maui.Controls.Xaml.UnitTests; | ||
|
|
||
| // Regression test for https://github.com/dotnet/maui/issues/18055 | ||
| // XamlC must use Dup (not Ldarg_0) before ldvirtftn when wiring virtual event handlers | ||
| // inside a DataTemplate, so the correct vtable object is used for virtual dispatch. | ||
| public partial class Maui18055 : ContentPage | ||
| { | ||
| public Maui18055() => InitializeComponent(); | ||
|
|
||
| public int BaseForVirtualClicked; | ||
| protected virtual void HandleVirtualClicked(object sender, EventArgs e) => BaseForVirtualClicked++; | ||
|
|
||
| [Collection("Issue")] | ||
| public class Tests | ||
| { | ||
| [Theory] | ||
| [XamlInflatorData] | ||
| // Verifies that a virtual handler wired inside a DataTemplate dispatches to the override, | ||
| // not the base class. Without the fix, XamlC emitted Ldarg_0 (the anonymous DataTemplate | ||
| // class) as the ldvirtftn vtable source — causing wrong dispatch on JIT and a hard crash | ||
| // on iOS/macOS Full AOT. | ||
| internal void VirtualHandlerInDataTemplateCallsOverride(XamlInflator inflator) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [minor] Regression Prevention - Only XamlInflator.XamlC exercises the IL fix. [XamlInflatorData] drives this theory with all three inflators, but:
Neither path would fail without the Dup fix. Only |
||
| { | ||
| var page = new SubMaui18055(inflator); | ||
| Assert.Equal(0, page.BaseForVirtualClicked); | ||
| Assert.Equal(0, page.OverrideClicked); | ||
|
|
||
| var template = (Microsoft.Maui.Controls.DataTemplate)page.Resources["virtualHandlerTemplate"]; | ||
| var element = (ElementWithEvent)template.CreateContent(); | ||
| element.SendClicked(); | ||
|
|
||
| // Override must be called; base must NOT be called. | ||
| Assert.Equal(1, page.OverrideClicked); | ||
| Assert.Equal(0, page.BaseForVirtualClicked); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| class SubMaui18055 : Maui18055 | ||
| { | ||
| public SubMaui18055(XamlInflator inflator) : base(inflator) { } | ||
|
|
||
| public int OverrideClicked; | ||
| protected override void HandleVirtualClicked(object sender, EventArgs e) => OverrideClicked++; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[moderate] XAML and Bindings - Secondary fix (static-abstract guard) has no regression test.
The
!methodDef.IsStaticguard preventsDup+ldvirtftnfor C# 11+ static abstract interface members. Without it, the old code would emitLdarg_0+ldvirtftnon anulldelegate target (the static branch pushes Ldnull above), causing a NullReferenceException at delegate-creation time. The guard correctly routes these toldftn, but there is no test covering it. Consider adding a test where a static abstract interface member is used as an event handler, to prevent a silent regression if this condition is ever relaxed.