[Catalyst] Fix: CollectionView Crash #29801

Tamilarasan-Paranthaman · 2025-06-03T12:51:56Z

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!

Description of Change

A crash was reported on Mac Catalyst with the following exception: ObjectDisposed_Generic for Foundation.NSIndexPath, occurring during selection in a CollectionView.
Based on the call stack, the crash originates from SelectableItemsViewController.ItemSelected, which attempts to use a disposed NSIndexPath. Although I was unable to reproduce the issue despite testing scenarios such as:
- Changing the ItemsSource
- Updating the collection while an item is selected
- Removing the selected item at runtime
- Performing rapid updates
I have provided a defensive fix by adding a null and handle check before invoking FormsSelectItem, FormsDeselectItem and SelectItem.
This change is expected to prevent the crash by defensively checking for a disposed NSIndexPath, without affecting expected selection behavior.

Issues Fixed

Fixes #29791

dotnet-policy-service · 2025-06-03T12:52:20Z

Hey there @@Tamilarasan-Paranthaman! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

Copilot

Pull Request Overview

This PR adds defensive null and handle checks around NSIndexPath usage in both iOS and Catalyst SelectableItemsViewController implementations to prevent crashes when an index path has been disposed.

Added guards in ItemSelected and ItemDeselected methods to return early if indexPath is null or its handle is zero.
Added the same guard in the internal SelectItem method before accessing index.Section and index.Item.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/Controls/src/Core/Handlers/Items2/iOS/SelectableItemsViewController2.cs	Added null/handle checks in selection callbacks and `SelectItem`
src/Controls/src/Core/Handlers/Items/iOS/SelectableItemsViewController.cs	Added null/handle checks in selection callbacks and `SelectItem`

Comments suppressed due to low confidence (1)

src/Controls/src/Core/Handlers/Items2/iOS/SelectableItemsViewController2.cs:28

Add a unit or UI test to simulate a null or disposed NSIndexPath scenario to verify these guards prevent crashes as intended.

if (indexPath is null || indexPath.Handle == IntPtr.Zero)

Copilot · 2025-06-03T12:53:43Z

src/Controls/src/Core/Handlers/Items2/iOS/SelectableItemsViewController2.cs

 		// _Only_ called if the user initiates the selection change; will not be called for programmatic selection
 		public override void ItemSelected(UICollectionView collectionView, NSIndexPath indexPath)
 		{
+			if (indexPath is null || indexPath.Handle == IntPtr.Zero)


The same null-and-handle check is duplicated across methods—consider extracting this into a private helper (e.g., bool IsValidIndexPath(NSIndexPath path)) to reduce duplication and ease future maintenance.

Suggested change

if (indexPath is null || indexPath.Handle == IntPtr.Zero)

if (!IsValidIndexPath(indexPath))

Copilot · 2025-06-03T12:53:44Z

src/Controls/src/Core/Handlers/Items/iOS/SelectableItemsViewController.cs

 		// _Only_ called if the user initiates the selection change; will not be called for programmatic selection
 		public override void ItemSelected(UICollectionView collectionView, NSIndexPath indexPath)
 		{
+			if (indexPath is null || indexPath.Handle == IntPtr.Zero)


Similar guard logic is repeated here—extract into a shared helper method to keep the code DRY and make future updates simpler.

Suggested change

if (indexPath is null || indexPath.Handle == IntPtr.Zero)

if (!IsValidIndexPath(indexPath))

Good suggestion IsValidIndexPath can be used for this check.

I think IsValidIndexPath is only on ListViewRenderer?

One thing I'm wondering is if we should re-evalute the indexes when they are used inside of PerformBatchUpdates

like in this code

CollectionView.PerformBatchUpdates(null, _ => { CollectionView.SelectItem(index, true, UICollectionViewScrollPosition.None); CollectionView.CellForItem(index)?.UpdateSelectedAccessibility(true); });

if the index should be retrieved a second time and if it's changed at all then we don't process the update

@PureWeen The case you mentioned could occur if the ItemsSource is updated immediately after setting the SelectedItem property, as shown in the example below. In such scenarios, during the PerformBatchUpdates call, the CollectionView items may be modified, and the index might no longer exist in the collection. In this case, we do not proceed with updating the collection.

However, the originally reported issue is unrelated to that scenario. As seen in the stack trace, the issue originates from the ItemSelected override method, which is triggered when the user taps to select an item. This indicates that the crash is likely occurring during user interaction. On the other hand, the SelectItem method you referenced is only invoked during programmatic selection, which is not the case here.

@sheiksyedm, will the changes in this PR also fix the issue caused by the 'ItemsSource updated immediately' problem?

@bhavanesh2001 No, it will not fix the ItemsSource updated problem. This fix may only address the ObjectDisposedException observed in the reported stack trace.

https://github.com/dotnet/maui/pull/29801/files#r2125125141

I'm wondering if we should re-evaluate the index when the performbatchupdate method is called and if that's the better overall fix here?

Or maybe we should change the ordering?

CollectionView.SelectItem(index, true, UICollectionViewScrollPosition.None);

Is going to trigger a behavior change reaction in the code
Would it make sense to have it like this instead

CollectionView.CellForItem(index)?.UpdateSelectedAccessibility(true); CollectionView.SelectItem(index, true, UICollectionViewScrollPosition.None);

I think IsValidIndexPath is only on ListViewRenderer?

One thing I'm wondering is if we should re-evalute the indexes when they are used inside of PerformBatchUpdates

like in this code

CollectionView.PerformBatchUpdates(null, _ => { CollectionView.SelectItem(index, true, UICollectionViewScrollPosition.None); CollectionView.CellForItem(index)?.UpdateSelectedAccessibility(true); });

if the index should be retrieved a second time and if it's changed at all then we don't process the update

We have this one https://github.com/dotnet/maui/blob/main/src/Compatibility/Core/src/iOS/CollectionView/IndexPathHelpers.cs#L36

Well thats Compatibility , this is the one I think we use on the CV

maui/src/Controls/src/Core/Handlers/Items/iOS/IndexPathHelpers.cs

Line 37 in 9894e0f

public static bool IsIndexPathValid(this IItemsViewSource source, NSIndexPath indexPath)

rmarinho · 2025-06-03T14:12:50Z

This seems to me that is hiding the issue, for some reason we are trying to deselect and disposed cell, I wonder if we can trying to figure out better what is calling it

rmarinho · 2025-06-03T17:04:03Z

/azp run MAUI-UITests-public

azure-pipelines · 2025-06-03T17:04:16Z

Azure Pipelines successfully started running 1 pipeline(s).

Tamilarasan-Paranthaman · 2025-06-04T09:24:24Z

This seems to me that is hiding the issue, for some reason we are trying to deselect and disposed cell, I wonder if we can trying to figure out better what is calling it

@rmarinho, as mentioned in the PR description, I was unfortunately unable to reproduce the reported issue. This change is intended as a temporary solution based solely on the call stack details, rather than a proper fix.

bhavanesh2001 · 2025-06-04T18:52:51Z

src/Controls/src/Core/Handlers/Items/iOS/SelectableItemsViewController.cs


 		// Called by Forms to mark an item selected 
 		internal void SelectItem(object selectedItem)
 		{


To extend on @PureWeen’s comment here: #29801 (comment)
we can be more cautious when performing PerformBatchUpdates(...) by adding guards to ensure the selection logic applies to the current and valid ItemsSource.

Something like this might help avoid re-selecting stale or unintended items if the ItemsSource has changed between scheduling and executing the batch update:

internal void SelectItem(object selectedItem) { var originalSource = ItemsView.ItemsSource; var index = GetIndexForItem(selectedItem); if (index.Section > -1 && index.Item > -1) { // Ensure the selected index is updated after the collection view's items generation is completed CollectionView.PerformBatchUpdates(null, _ => { // Exit if the ItemsSource reference no longer matches the one captured at invocation. if (!ReferenceEquals(ItemsView.ItemsSource, originalSource)) { return; } // Recalculate the index for the selectedItem now that the collection may have changed.(Adding, deleting etc..) var updatedIndex = GetIndexForItem(selectedItem); if (updatedIndex.Section < 0 || updatedIndex.Item < 0) { return; } // Retrieve the current item at that index and verify it still equals the intended selection. var liveItem = GetItemAtIndex(updatedIndex); if (!Equals(liveItem, selectedItem)) { return; } CollectionView.SelectItem(index, true, UICollectionViewScrollPosition.None); CollectionView.CellForItem(index)?.UpdateSelectedAccessibility(true); }); } }

jsuarezruiz · 2025-06-25T07:43:31Z

@Tamilarasan-Paranthaman Could you rebase to fix the conflicts? Thanks in advance.

Tamilarasan-Paranthaman · 2025-06-25T08:00:22Z

@Tamilarasan-Paranthaman Could you rebase to fix the conflicts? Thanks in advance.

@jsuarezruiz, we reverted that implementation in PR #29827, so this PR can be closed.

Fixed ObjectDisposedException

e6c75ff

Tamilarasan-Paranthaman marked this pull request as ready for review June 3, 2025 12:52

Copilot AI review requested due to automatic review settings June 3, 2025 12:52

Tamilarasan-Paranthaman requested a review from a team as a code owner June 3, 2025 12:52

dotnet-policy-service bot added the community ✨ Community Contribution label Jun 3, 2025

Tamilarasan-Paranthaman requested review from mattleibow and rmarinho June 3, 2025 12:52

dotnet-policy-service bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Jun 3, 2025

Copilot AI reviewed Jun 3, 2025

View reviewed changes

bhavanesh2001 reviewed Jun 4, 2025

View reviewed changes

Tamilarasan-Paranthaman closed this Jun 25, 2025

github-actions bot locked and limited conversation to collaborators Jul 25, 2025

	if (indexPath is null \|\| indexPath.Handle == IntPtr.Zero)
	if (!IsValidIndexPath(indexPath))

[Catalyst] Fix: CollectionView Crash #29801

[Catalyst] Fix: CollectionView Crash #29801

Uh oh!

Conversation

Tamilarasan-Paranthaman commented Jun 3, 2025

Description of Change

Issues Fixed

Uh oh!

dotnet-policy-service bot commented Jun 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmarinho commented Jun 3, 2025

Uh oh!

rmarinho commented Jun 3, 2025

Uh oh!

azure-pipelines bot commented Jun 3, 2025

Uh oh!

Tamilarasan-Paranthaman commented Jun 4, 2025

Uh oh!

bhavanesh2001 Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsuarezruiz commented Jun 25, 2025

Uh oh!

Tamilarasan-Paranthaman commented Jun 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bhavanesh2001 Jun 4, 2025 •

edited

Loading