Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] ios ItemsView leak when using INotifyCollectionChanged and crash #14113

Closed
tmijieux opened this issue Apr 5, 2021 · 4 comments
Closed

Comments

@tmijieux
Copy link
Contributor

tmijieux commented Apr 5, 2021

Description

When an CollectionView(or any ItemsView)'s ItemsSource is an INotifyCollectionChanged( for instance a ObservableCollection), and if that CollectionView is removed from view hierarchy(not used anymore), while the ObservableCollection continue to be used after the first CollectionView is removed, then the Event handler registered by the CollectionView on the CollectionChanged event is never unregistered.
In some cases, the application will crash because of the remaining event handlers that should have been removed(see reproduction)

associated pull request: GH-14111

Steps to Reproduce

clone, build and launch minimal reproduction https://github.com/tmijieux/XFBugItemsViewController

  1. click "Fill" button (notice under the output tab of visual studio, there is 1 listener to the CollectionChanged events")
  2. Rotate device
  3. click "Fill" button (notice under the output tab of visual studio, there is now 2 listeners to the CollectionChanged events!")
  4. Rotate device again
  5. click "Fill" button (notice under the output tab of visual studio, there is now 3 listeners to the CollectionChanged events!")
  6. click "Empty" button

Expected Behavior

after step 1, 3, and 5 the number of listener to the CollectionChanged is 1 each time
after step 6 the application does not crash

Actual Behavior

after step 1, 3, and 5 the number of listener to the CollectionChanged is increased by 1 each time
after step 6 the application crashes

Basic Information

  • Version with issue: 5.0.0.2012

  • Platform Target Frameworks:

    • iOS: Xamarin.iOS and Xamarin.Mac SDK 14.14.2.5 (3836759d4)
  • NuGet Packages: Xamarin.Forms 5.0.0.2012, Xamarin.Essentials 1.6.1

  • Affected Devices: tested on iPad (6th generation) with iOS 14.4.2

Environment

  1. Visual Studio:
Show/Hide Visual Studio info
 Microsoft Visual Studio Community 2019
Version 16.9.3
VisualStudio.16.Release/16.9.3+31129.286
Microsoft .NET Framework
Version 4.8.04084

Installed Version: Community

Visual C++ 2019   00435-60000-00000-AA508
Microsoft Visual C++ 2019

.NET Core Debugging with WSL 2   1.0
.NET Core Debugging with WSL 2

ASP.NET and Web Tools 2019   16.9.693.2781
ASP.NET and Web Tools 2019

ASP.NET Core Razor Language Services   16.1.0.2112521+5741df381174d72f08e3632bb99f52e8635b6a1a
Provides languages services for ASP.NET Core Razor.

ASP.NET Web Frameworks and Tools 2019   16.9.693.2781
For additional information, visit https://www.asp.net/

Azure App Service Tools v3.0.0   16.9.693.2781
Azure App Service Tools v3.0.0

Azure Functions and Web Jobs Tools   16.9.693.2781
Azure Functions and Web Jobs Tools

C# Tools   3.9.0-6.21160.10+59eedc33d35754759994155ea2f4e1012a9951e3
C# components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used.

Common Azure Tools   1.10
Provides common services for use by Azure Mobile Services and Microsoft Azure Tools.

Extensibility Message Bus   1.2.6 (master@34d6af2)
Provides common messaging-based MEF services for loosely coupled Visual Studio extension components communication and integration.

IntelliCode Extension   1.0
IntelliCode Visual Studio Extension Detailed Info

Microsoft Azure Tools   2.9
Microsoft Azure Tools for Microsoft Visual Studio 2019 - v2.9.40218.1

Microsoft Continuous Delivery Tools for Visual Studio   0.4
Simplifying the configuration of Azure DevOps pipelines from within the Visual Studio IDE.

Microsoft JVM Debugger   1.0
Provides support for connecting the Visual Studio debugger to JDWP compatible Java Virtual Machines

Microsoft Library Manager   2.1.113+g422d40002e.RR
Install client-side libraries easily to any web project

Microsoft MI-Based Debugger   1.0
Provides support for connecting Visual Studio to MI compatible debuggers

Microsoft Visual C++ Wizards   1.0
Microsoft Visual C++ Wizards

Microsoft Visual Studio Tools for Containers   1.1
Develop, run, validate your ASP.NET Core applications in the target environment. F5 your application directly into a container with debugging, or CTRL + F5 to edit & refresh your app without having to rebuild the container.

Microsoft Visual Studio VC Package   1.0
Microsoft Visual Studio VC Package

Mono Debugging for Visual Studio   16.9.7 (df23ba6)
Support for debugging Mono processes with Visual Studio.

NuGet Package Manager   5.9.0
NuGet Package Manager in Visual Studio. For more information about NuGet, visit https://docs.nuget.org/

ProjectServicesPackage Extension   1.0
ProjectServicesPackage Visual Studio Extension Detailed Info

SQL Server Data Tools   16.0.62103.10080
Microsoft SQL Server Data Tools

TypeScript Tools   16.0.30201.2001
TypeScript Tools for Microsoft Visual Studio

Visual Basic Tools   3.9.0-6.21160.10+59eedc33d35754759994155ea2f4e1012a9951e3
Visual Basic components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used.

Visual F# Tools   16.9.0-beta.21102.9+7ce7132f1459095e635194d09d6f73265352029a
Microsoft Visual F# Tools

Visual Studio Code Debug Adapter Host Package   1.0
Interop layer for hosting Visual Studio Code debug adapters in Visual Studio

Visual Studio Container Tools Extensions   1.0
View, manage, and diagnose containers within Visual Studio.

Visual Studio Tools for Containers   1.0
Visual Studio Tools for Containers

VisualStudio.DeviceLog   1.0
Information about my package

VisualStudio.Foo   1.0
Information about my package

VisualStudio.Mac   1.0
Mac Extension for Visual Studio

Xamarin   16.9.000.273 (d16-9@1bba9e0)
Visual Studio extension to enable development for Xamarin.iOS and Xamarin.Android.

Xamarin Designer   16.9.0.316 (remotes/origin/d16-9@fdbf64026)
Visual Studio extension to enable Xamarin Designer tools in Visual Studio.

Xamarin Templates   16.9.68 (8e9b569)
Templates for building iOS, Android, and Windows apps with Xamarin and Xamarin.Forms.

Xamarin.Android SDK   11.2.2.1 (d16-9/877f572)
Xamarin.Android Reference Assemblies and MSBuild support.
    Mono: 5e9cb6d
    Java.Interop: xamarin/java.interop/d16-9@54f8c24
    ProGuard: Guardsquare/proguard/v7.0.1@912d149
    SQLite: xamarin/sqlite/3.34.1@daff8f4
    Xamarin.Android Tools: xamarin/xamarin-android-tools/d16-9@d210f11


Xamarin.iOS and Xamarin.Mac SDK   14.14.2.5 (3836759d4)
Xamarin.iOS and Xamarin.Mac Reference Assemblies and MSBuild support.

Build Logs

Screenshots

Reproduction Link

https://github.com/tmijieux/XFBugItemsViewController

Workaround

to avoid the crash, instead of calling the event handler directly with CollectionChanged?.Invoke()
you can iterate on invocation list with a try/catch each time

to avoid the leak the solution seems to renew the viewmodel / observablecollection, each time the view is changed, so that the old viewmodel can be garbage collected , and then the ObservableItemsSource as well.

@jtorvald
Copy link
Contributor

jtorvald commented Apr 7, 2021

I'm quite sure that I commented on this yesterday?
I already added a PR for this issue: #14076 together with some other events that were dangling.

@tmijieux
Copy link
Contributor Author

tmijieux commented Apr 7, 2021

@jtorvald you commented on the pull request, this is the associated issue. You probably got a little confused.

Are you sure your pull request is exactly fixing the same issue than mine ? i looked at your pull request inside the ItemsViewController you only update the Dispose method, and add that UnbindCells methods, but when Dispose is called on the ItemsViewController, it is already too late (in my issue it has already has been leaked, and is already not recoverable /not referenced anymore from the ItemsViewController) and it has nothing to do with cells ...

@jtorvald
Copy link
Contributor

jtorvald commented Apr 7, 2021

@tmijieux thanks Thomas, definitely confused!

Apologies, I thought it was the same issue. Looking at your code I believe your fix solves a different issue.
I had trouble debugging this in the XF Gallery because the sample code also has memory leaks. I tried to fix them for the collectionview in my pr.

@jsuarezruiz jsuarezruiz added this to New in Triage via automation Apr 28, 2021
@rachelkang rachelkang added e/2 🕑 2 and removed s/unverified New report that has yet to be verified labels Apr 30, 2021
@rachelkang rachelkang moved this from New to Ready For Work in Triage Apr 30, 2021
@tmijieux
Copy link
Contributor Author

fixed by GH-14111

Triage automation moved this from Ready For Work to Closed Jul 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Triage
  
Closed
Development

No branches or pull requests

4 participants