Skip to content
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

ListView crashes when ISelectionInfo is used with Extended selection #10025

Open
sjb-sjb opened this issue Sep 27, 2024 · 4 comments
Open

ListView crashes when ISelectionInfo is used with Extended selection #10025

sjb-sjb opened this issue Sep 27, 2024 · 4 comments
Labels
bug Something isn't working needs-triage Issue needs to be triaged by the area owners

Comments

@sjb-sjb
Copy link

sjb-sjb commented Sep 27, 2024

Describe the bug

An ArgumentOutOfRangeException is thrown when using ListView extended selection with ISelectionInfo.

I originally listed this under #8684 but it would seem to be a separate bug (or reportably separately) since it does not involve a collection changed notification. I guess the root cause could be the same.

Steps to reproduce the bug

  1. Download the WinUI 3 Gallery and compile the WinUiGallery solution.

  2. In ControlPages / ListViewPage.xaml.cs, add "using Microsoft.UI.Xaml.Data;" at the top of the file and add the following collection class at the bottom of the namespace. This is an ObservableCollection implementing ISelectionInfo (not completely, but sufficiently for the demonstration).

    public class ObservableCollectionSelector<T> : ObservableCollection<T>, ISelectionInfo
    {
        public ObservableCollectionSelector(IEnumerable<T> source) : base(source) { }
        private IList<ItemIndexRange> Selection = new List<ItemIndexRange>();

        public void SelectRange(ItemIndexRange itemIndexRange)
        {
            //Debug.WriteLine($"Select {itemIndexRange.FirstIndex}:{itemIndexRange.LastIndex}");
            if (this.Selection.Any(iir => iir.FirstIndex <= itemIndexRange.LastIndex + 1 && iir.LastIndex >= itemIndexRange.FirstIndex - 1)) { throw new NotSupportedException("failed select"); }
            this.Selection.Add(itemIndexRange);
        }
        public void DeselectRange(ItemIndexRange itemIndexRange)
        {
            //Debug.WriteLine($"Deselect {itemIndexRange.FirstIndex}:{itemIndexRange.LastIndex}");
            if (!this.Selection.Any(iir => iir.FirstIndex == itemIndexRange.FirstIndex && iir.LastIndex == itemIndexRange.LastIndex)) { throw new NotSupportedException("failed deselect"); }
            this.Selection.Remove(itemIndexRange);
        }
        public bool IsSelected(int index)
        {
            return this.Selection.Any(iir => iir.FirstIndex <= index && iir.LastIndex >= index);
        }
        public IReadOnlyList<ItemIndexRange> GetSelectedRanges()
        {
            //Debug.Assert(!this.Selection.Any((iir, i) => i > 0 && iir.FirstIndex <= this.Selection[i - 1].FirstIndex + 1));
            return this.Selection.AsReadOnly();
        }
    }
  1. Around line 59, change Control2.ItemsSource = await Contact.GetContactsAsync(); so that it says Control2.ItemsSource = new ObservableCollectionSelector&lt;Contact&gt;( await Contact.GetContactsAsync());

  2. Run the app, select Collections on the left and then ListView. Scroll down to the second ListView ("ListView with Selection Support") and select Extended as the selection type. Click on the first entry ("Kendal Collins") and then CTRL-Click on the third entry ("Vance DeLeon"). Then release the control key and click on the 5th entry ("Amber Rodriguez").

Expected behavior

Expected: the two previously selected items should be deselected and the new one should be selected. Actual: no change in visual state, and a crash with an ArgumentOutOfRange exception in WinRT.Runtime.dll. If you uncomment the WriteLine statements, it shows that index 0 is deselected then the crash occurs.

The details of the exception are:
Exception thrown: 'System.ArgumentOutOfRangeException' in WinRT.Runtime.dll
Exception thrown at 0x00007FFFC0F6B699 (KernelBase.dll) in WinUIGallery.exe: WinRT originate error - 0x8000000B : 'This collection cannot work with indices larger than Int32.MaxValue - 1 (0x7FFFFFFF - 1). (Parameter 'index')'.
Microsoft.ui.xaml.dll!00007FFF23716C7D: 8000000B - E_BOUNDS

Screenshots

No response

NuGet package version

None

Windows version

No response

Additional context

WindowsAppSDK 1.6.240829007 and I happen to be using Windows 10, 22H2 build 19045.4894

@sjb-sjb sjb-sjb added the bug Something isn't working label Sep 27, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Sep 27, 2024
@sjb-sjb
Copy link
Author

sjb-sjb commented Sep 30, 2024

This bug makes it impossible to use virtualizing ListView with multiple selection. A blocker for my project! Any thoughts on fixing this?

@sjb-sjb sjb-sjb changed the title ListView crashes with ArgumentOutOfRangeException when ISelectionInfo is used ListView crashes when ISelectionInfo is used with Extended selection Oct 1, 2024
@w-ahmad
Copy link

w-ahmad commented Oct 4, 2024

it looks like something is wrong with the List<ItemIndexRange> data structure, I tried HashSet<ItemIndexRange> and it worked.

@sjb-sjb
Copy link
Author

sjb-sjb commented Oct 5, 2024

@w-ahmad are you saying that the List&lt;T&gt; implementation has a bug in it? That would be quite significant considering how fundamental that class is in .NET —-

Edit: HashSet does not implement IReadOnlyList, so, to return the list from the HashSet in GetSelectedRanges you will have needed to make a copy of it, for example with ToList. See my post below.

@sjb-sjb
Copy link
Author

sjb-sjb commented Oct 5, 2024

After some experimenting I concluded that most likely the ListView is invalidating the selection iterator by deselecting while iterating.

First step was that I changed IList to List in the declaration of Selection. This way it is not necessary to call AsReadOnly() in GetSelectedRanges.

Second step I compared returning Selection to returning Selection.ToList() from GetSelectedRanges and tracing the sequence of calls. If Selection is returned then it first gets the list of selected ranges and then crashes after deselecting index 0. But if Selection.ToList() is returned then after deselecting index 0, it continues and deselects index 2, then selects index 4.

While the exception thrown is not the usual one saying that the iterator has been invalidated, the other symptoms tell the story. The deselection of index 0 changes the Selection list and thereby certainly invalidates any iterator that was obtained earlier. However if a copy of Selection is made then changes to Selection do not invalidate the iterator of that copy.

The selection code in ListView should be reviewed and likely fixed so that during deselection it always pulls the first element from the list, or uses a buffer, or otherwise avoids invalidating its iterator.

Here is the updated collection code. Switch the commented FAILS/SUCCEEDS lines in GetSelectedRanges and compare the outputs.

public class ObservableCollectionSelector<T> : ObservableCollection<T>, ISelectionInfo
{
    public ObservableCollectionSelector(IEnumerable<T> source) : base(source) { }
    private List<ItemIndexRange> Selection = new List<ItemIndexRange>();
    // Alternative: private IList<ItemIndexRange> Selection = new List<ItemIndexRange>();
    // Alternative: private HashSet<ItemIndexRange> Selection = new HashSet<ItemIndexRange>();

    public void SelectRange(ItemIndexRange itemIndexRange)
    {
        Debug.WriteLine($"Selecting {itemIndexRange.FirstIndex}:{itemIndexRange.LastIndex}");
        if (this.Selection.Any(iir => iir.FirstIndex <= itemIndexRange.LastIndex + 1 && iir.LastIndex >= itemIndexRange.FirstIndex - 1)) { throw new NotSupportedException("failed select"); }
        this.Selection.Add(itemIndexRange);
        Debug.WriteLine($"Selected {itemIndexRange.FirstIndex}:{itemIndexRange.LastIndex}");
    }
    public void DeselectRange(ItemIndexRange itemIndexRange)
    {
        Debug.WriteLine($"Deselecting {itemIndexRange.FirstIndex}:{itemIndexRange.LastIndex}");
        if (!this.Selection.Any(iir => iir.FirstIndex == itemIndexRange.FirstIndex && iir.LastIndex == itemIndexRange.LastIndex)) { throw new NotSupportedException("failed deselect"); }
        this.Selection.Remove(itemIndexRange);
        Debug.WriteLine($"Deselected {itemIndexRange.FirstIndex}:{itemIndexRange.LastIndex}");
    }
    public bool IsSelected(int index)
    {
        return this.Selection.Any(iir => iir.FirstIndex <= index && iir.LastIndex >= index);
    }
    public IReadOnlyList<ItemIndexRange> GetSelectedRanges()
    {
        //Debug.Assert(this.Selection.None((iir, i) => i > 0 && iir.FirstIndex <= this.Selection[i - 1].FirstIndex + 1));

        Debug.WriteLine($"Getting selected ranges");
        IReadOnlyList<ItemIndexRange> selectedRanges = this.Selection; // FAILS 
        //IReadOnlyList<ItemIndexRange> selectedRanges = this.Selection.ToList(); // SUCCEEDS
        Debug.WriteLine($"Got {selectedRanges.Count} selected ranges");
        return selectedRanges;
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-triage Issue needs to be triaged by the area owners
Projects
None yet
Development

No branches or pull requests

2 participants