This is Xamarin's bug tracking system. For product support, please use the support links listed in your Xamarin Account.
Bug 44074 - OnDetachingFrom for Behavior is never called, therefore memory can leak due to event handlers not being unbound
Summary: OnDetachingFrom for Behavior is never called, therefore memory can leak due t...
Status: RESOLVED FIXED
Alias: None
Product: Forms
Classification: Xamarin
Component: Forms (show other bugs)
Version: 2.3.2
Hardware: Macintosh Mac OS
: --- normal
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2016-09-07 23:25 UTC by Cliff Cawley
Modified: 2017-05-11 09:41 UTC (History)
8 users (show)

See Also:
Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Sample to reproduce bug (54.77 KB, application/zip)
2016-09-07 23:25 UTC, Cliff Cawley
Details

Description Cliff Cawley 2016-09-07 23:25:23 UTC
Created attachment 17370 [details]
Sample to reproduce bug

== Issue ==
OnDetachingFrom is never called when the view is no longer active, therefore memory leaks can occur due to event handlers not being unbound or other code not being executed.

== Expected ==
OnDetachingFrom is called when the page goes out of context.

== Steps to reproduce ==
Open Solution and run either Droid or iOS. Click the button and notice the Debug log for OnAttached. Click the next button and notice no OnDetachingFrom

== Additional Notes ==
MainPage assignment is used in the example but the same thing happens if you use Navigation and push a view, then tap back.
Comment 1 Rui Marinho 2016-10-04 11:56:24 UTC
Still in 2.3.3
Comment 2 adrianknight89 2016-11-20 22:54:29 UTC
See https://github.com/xamarin/Xamarin.Forms/pull/553
Comment 3 Rui Marinho 2016-12-06 12:19:29 UTC
Should be fixed in 2.3.4-pre2
Comment 4 Ione Souza Junior 2016-12-08 19:36:25 UTC
As alternative way, you can implement destructor method in your behavior class and manually call OnDetachingFrom method.
Comment 5 Parmendra Kumar 2017-02-02 19:04:14 UTC
I have checked this issue with Xamarin.Forms 2.3.4.184-pre1 and this issue has been fixed in iOS and Still getting same issue with Android.

Screencast: https://www.screencast.com/t/fsQLsY3oQHc

ApplicationOutput: https://gist.github.com/Parmendrak/f91c3b69869d455f17f1119378395990

Hence reopened this issue with respect to Android. 


Thanks.
Comment 6 David Waterman 2017-02-13 11:28:24 UTC
The changes made for Bug 44074 and 51503 release in 2.3.4.184-pre2 are causing crashes when the ~ViewElement finalizer code executes on a UWP App; I haven't tried this on other platforms but I suspect they will also have problems.
The issue arises when views with bound triggers are garbage collected.
The changes to ~ViewElement cause calls to event handlers from the finalizer thread leading to COM exceptions. 
According to Miscrosoft https://msdn.microsoft.com/en-us/library/system.object.finalize(v=vs.110).aspx: "The Finalize method is used to perform cleanup operations on unmanaged resources".
These changes are cleaning up managed resources.

Here is an example crash:
System.Runtime.InteropServices.COMException was unhandled
  ErrorCode=-2147417842
  HResult=-2147417842
  Message=The application called an interface that was marshalled for a different thread. (Exception from HRESULT: 0x8001010E (RPC_E_WRONG_THREAD))
  Source=mscorlib
  StackTrace:
       at System.StubHelpers.EventArgsMarshaler.CreateNativePCEventArgsInstance(String name)
       at System.Runtime.InteropServices.WindowsRuntime.PropertyChangedEventArgsMarshaler.ConvertToNative(PropertyChangedEventArgs managedArgs)
       at Xamarin.Forms.BindableObject.OnPropertyChanged(String propertyName)
       at Xamarin.Forms.Element.OnPropertyChanged(String propertyName)
       at Xamarin.Forms.BindableObject.ClearValue(BindableProperty property, Boolean checkaccess)
       at Xamarin.Forms.BindingCondition.TearDown(BindableObject bindable)
       at Xamarin.Forms.TriggerBase.OnDetachingFrom(BindableObject bindable)
       at Xamarin.Forms.TriggerBase.Xamarin.Forms.IAttachedObject.DetachFrom(BindableObject bindable)
       at Xamarin.Forms.AttachedCollection`1.OnDetachingFrom(BindableObject bindable)
       at Xamarin.Forms.VisualElement.Finalize()
	   
And here is sample code to reproduce the crash; from a UWP application displaying Page1.cs press the Clear button followed by the Garbage button to force a garbage collection and crash.	   

using System;
using System.Collections.ObjectModel;
using System.Collections.Specialized;

using Xamarin.Forms;

namespace App4
{
    public class Page1 : ContentPage
    {
        StackLayout _layout = new StackLayout();
        StackLayout _itemsPanel = new StackLayout();
        DataTemplate _itemTemplate = new DataTemplate(CreateBoxView);

        public Page1()
        {
            var viewModel = new ObservableCollection<Item>()
            {
                new Item() { Subject = 65 },
                new Item() { Subject = 66 },
                new Item() { Subject = 65 },
            };

            viewModel.CollectionChanged += OnCollectionChanged;

            _itemsPanel.BindingContext = viewModel;

            foreach (var item in viewModel)
            {
                _itemTemplate.SetValue(BindingContextProperty, item);
                var view = (View)_itemTemplate.CreateContent();
                _itemsPanel.Children.Add(view);
            }
            _layout.Children.Add(_itemsPanel);

            var clearButton = new Button() { Text = "Clear", Command = new Command((o) => viewModel.Clear()) };
            _layout.Children.Add(clearButton);

            var collectButton = new Button() { Text = "Garbage", Command = new Command((o) => GC.Collect()) };
            _layout.Children.Add(collectButton);

            Content = _layout;
        }

        private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
        {
            if (e.Action == NotifyCollectionChangedAction.Reset)
            {
                // reset the list
                _itemsPanel.Children.Clear();
            }
        }

        private static object CreateBoxView()
        {
            var boxView1 = new BoxView() { HeightRequest = 100, Color = new Color(0.55, 0.23, 0.147) };
            var setter1 = new Setter() { Property = BoxView.ColorProperty, Value = "#FF2879DD" };
            var trigger1 = new DataTrigger(typeof(BoxView)) { Binding = new Binding("Subject"), Value = 65 };
            trigger1.Setters.Add(setter1);
            boxView1.Triggers.Add(trigger1);
            return boxView1;
        }

        public class Item
        {
            public int Subject { get; set; }
        }
    }
}
Comment 7 Rui Marinho 2017-03-23 19:24:59 UTC
This issue is actualy fixed, on Android the GC has a diferent behavior so you must click more times and you eventually will get the Detached being called.
Comment 8 Rui Marinho 2017-03-23 19:28:58 UTC
David Waterman please open a new bug with a reproducion exemple and will triage and take a look. 

Thanks.
Comment 9 David Waterman 2017-03-24 12:35:14 UTC
My point is that the fix made for the original bug report is will cause crashes in UWP programs. Simply upgrading from XF 2.3.3 to 2.3.4.184-pre2 caused my UWP to crash due to the changes made for this bug report in the ~ViewElement.

The example code I pasted in comment 6 will demonstrate the problem if you use it build a simple UWP app with 2.3.4.184-pre2.

Note You need to log in before you can comment on or make changes to this bug.