Microsoft-ui-xaml: SwipeItems.Clear() throws an AccessViolationException if the items are already in use

[Sergio0694]

Describe the bug

As the title says, trying to call SwipeItems.Clear() throws an AccessViolationException, if those items have already been used in a visible template.

Steps to reproduce the bug

Steps to reproduce the behavior:

1. Create a resource dictionary with code behind, reference it in App.xaml
2. Create a SwipeItems and a SwipeItem object in the XAML part of that resource dictionary:

xmlns:controls="using:Microsoft.UI.Xaml.Controls"
<controls:SwipeItems x:Name="ItemLeftActions" x:Key="ItemLeftActions"/>
<controls:SwipeItem x:Name="FirstItemAction"/>

1. Create the following method in the code behind of that resource dictionary:

public void SetupActions()
{
    ItemLeftActions.Clear();
    ItemLeftActions.Mode = SwipeMode.Execute;
    ItemLeftActions.Add(FirstItemAction);
}

1. Setup a way to invoke that SetupActions method (an easy way is to use the GalaSoft.MVVMLight package, setup a message in the constructor of that dictionary that invokes that SetupActions method, and then send that message from anywhere else in the app when you want to call that method in the dictionary).
2. Reference that shared SwipeItems instance from a data template used in a ListView:

<DataTemplate>
    <SwipeControl LeftItems="{StaticResource CommentLeftActions}">
        <Grid Background="Green" Height="80"/>
    </SwipeControl>
</DataTemplate>

1. Invoke that SetupActions _before any of the items to display are loaded_. This works fine.
2. Observe that the swipe action is correctly available in all the visible item templates in the ListView
3. Invoke the SetupActions method again - CRASH

NOTE: the same issue should also happen without using a resource dictionary, but just by declaring items directly from within a template, and then trying to remove/modify them after the template has been displayed. Though I personally haven't tried this alternative repro yet.

Expected behavior

The SwipeItems objects should correctly clear their items and receive the new ones, ready to be used in the SwipeControls referencing those swipe items through StaticResource.

Screenshots

Version Info

Windows 10: 10.0.18362.175 x64
NuGet package version: WinUI: 2.1.190606001

| Windows 10 version | Saw the problem? |
| :--------------------------------- | :-------------------- |
| Insider Build (xxxxx) | |
| May 2019 Update (18362) | Yes/No |
| October 2018 Update (17763) | |
| April 2018 Update (17134) | |
| Fall Creators Update (16299) | |
| Creators Update (15063) | |
| Anniversary Update (14393) | |

| Device form factor | Saw the problem? |
| :-------------------- | :------------------- |
| Desktop | Yes |
| Mobile | |
| Xbox | |
| Surface Hub | |
| IoT | |

Additional context

The app is targeting the SDK 17763 as both minimum and target.
The reason for clearing/reinitializing the SwipeActions is that the app I'm working on (Legere for Reddit) includes the ability to customize the swipe actions, and right now due to this crash I have to tell the users to restart the app every time the actions are updated in the settings, as attempting to update them when the SwipeControls using them are already visible, results in this crash.
Also, the same crash happens when trying to use RemoveAt and Remove too.
In general, any change in the items after they've been displayed, results in this crash.

[YuliKl]

@kikisaints, can you clarify whether this is a supported scenario?

[kikisaints]

Hey again @Sergio0694 - I own the Swipe control, by the way, in case you haven't noticed yet 😄

Just wanted to let you know I'm going to try and test this myself - but in the meantime, have you tried/can you use a ThemeResource instead of a StaticResource? I know typically StaticResources don't update as easily.

[Sergio0694]

Hey @kikisaints - yeah I just noticed ahahah
Glad the 3rd issue I opened about the SwipeControl (#890) has been assigned to someone else at least, so that I'm not just spamming you personally with them 🙈

I tried switching to ThemeResource as you suggested but unfortunately I get the same result - as soon as I call SwipeItems.Clear() after those items have already been referenced by a realized item template in the app, I get that AccessViolationException.

At least this is not a complete show stopper though, as I mentioned I'm just telling my users to restart the app for the changes in the swipe items to take effect. Still, I'd be curious to know if this is something that could be fixed. I mean, at the very least, an AccessViolationException is not something that one would ever expect to get from a library, especially because that specific exception can't even be handled at all in C# (I mean you can catch it, but then the process will still terminate).
Thanks again for your help 😊

[RBrid]

This is definitely a WinUI bug. Our code should never result in this kind of exception. If this is not a supported scenario, another exception like InvalidOperationException with a custom error message should be used.

[kikisaints]

Hey! Sorry for the prolonged delay on this! This scenario should be supported. There are cases where people would need to change the swipe items at runtime (app menu settings update, response to actions, customization of menus, etc.).

I believe using the Clear function is the logical solution to this need.

[Sergio0694]

Hey @kikisaints - thank you for the update!
Glad to hear you plan on eventually supporting this, looking forward to that! 😄

[chingucoding]

@kikisaints: I can take a look at that, if that is ok. :)

What should be default behavior instead of throwing that exception? Not throwing any exception and resume normally? :sweat_smile:

[kikisaints]

@chingucoding that's actually a decision that's up to @jevansaks, but to answer your question around what it should do - I imagine that it would clear the contents of the SwipeItems element that Clear was called on, and ideally not throw an exception 😃 .

The sticking point here would probably be around if someone called Clear() on a SwipeItems element, then never added any items back in, and then attempted to swipe. In that case I would expect to see nothing or a blank swipe item in it's stead.

[chingucoding]

@chingucoding that's actually a decision that's up to @jevansaks

Oh haha I did not know that. 😅
@jevansaks If you want me to look into this, you can assign me 😄

[jevansaks]

If you want me to look into this, you can assign me 😄

Happily!

[msft-github-bot]

:tada:This issue was addressed in #1383, which has now been successfully released as Microsoft.UI.Xaml v2.3.191007001-prerelease.:tada:

Handy links:

• Release Notes
• Official Nuget Release

[msft-github-bot]

:tada:This issue was addressed in #1383, which has now been successfully released as Microsoft.UI.Xaml v2.3.191129002.:tada:

Handy links:

• Release Notes
• Official Nuget Release