Where to unsubscribe events in an attached property?

Question

I have an attached property to use in a datagrid to can use the SelectedItems in my view model. The code is this:

public class DataGridSelectedItemsAttachedProperty
    {
        #region SelectedItems
        ///
        /// SelectedItems Attached Dependency Property
        ///
        public static readonly DependencyProperty SelectedItemsProperty =
        DependencyProperty.RegisterAttached("SelectedItems", typeof(IList),
        typeof(DataGridSelectedItemsAttachedProperty),
        new FrameworkPropertyMetadata(null, FrameworkPropertyMetadataOptions.BindsTwoWayByDefault,
        new PropertyChangedCallback(OnSelectedItemsChanged)));

        public static IList GetSelectedItems(DependencyObject d)
        {
            return (IList)d.GetValue(SelectedItemsProperty);
        }

        public static void SetSelectedItems(DependencyObject d, IList value)
        {
            d.SetValue(SelectedItemsProperty, value);
        }

        private static void OnSelectedItemsChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
        {
            DataGrid miDg = (DataGrid)d;
            miDg.SelectionChanged += dataGrid_SelectionChanged;
            miDg.Unloaded += dataGrid_Unloaded;
        }

        private static void dataGrid_SelectionChanged(object sender, SelectionChangedEventArgs e)
        {
            DataGrid miDg = (DataGrid)sender;
            //Get list box's selected items.
            IEnumerable miDgSelectedItems = miDg.SelectedItems;
            //Get list from model
            IList ModelSelectedItems = GetSelectedItems(miDg);

            //Update the model
            ModelSelectedItems.Clear();

            if (miDg.SelectedItems != null)
            {
                foreach (var item in miDg.SelectedItems)
                    ModelSelectedItems.Add(item);
            }
            SetSelectedItems(miDg, ModelSelectedItems);
        }


        private static void dataGrid_Unloaded(object sender, RoutedEventArgs e)
        {
            DataGrid miDg = sender as DataGrid;
            miDg.SelectionChanged -= dataGrid_SelectionChanged;
            miDg.Unloaded -= dataGrid_Unloaded;
        }
        #endregion
    }

The problem is that this datagrid is in a tab control the event unload is fired, so the event is unsubcribe and then the SelectedItems is not notified to the view model anymore.

So I would like to know how to solve this problem, perhaps unsubscribe the events in another place instead of the unload event?

Thanks.

Right. Really it was an example that I found and I think it is a good solution. But I am thinking that in this cases it is not needed to unsubcribe because the attached property has to live the same time than the user control, si when I close the user control is when the attached property will be recolected because no object is referencing it. So in this case I guess it is unneded too. — Álvaro García, Dec 31 '17 at 08:38

score 2 · Answer 1 · answered Apr 27 '20 at 13:33

I have faced the same question, but come to the conclustion that it is unnecessary to unsubscribe from events in this case (thanks for comments from Álvaro García and Blechdose that point me in this direction).

Actualy memory leaks because of an event handler are a one way problem. The cause of this problem is described here: https://stackoverflow.com/a/4526840/12797700. By using this code miDg.SelectionChanged += dataGrid_SelectionChanged; you add a link to the object that stores the dataGrid_SelectionChanged method into the miDg object. Because of that GC cannot remove the object that stores dataGrid_SelectionChanged method while the miDg object is alive.

However, the static object know nothing about the miDg object and GC can remove the miDg object even if the event is handled.

You can download the test project demonstrating this behavior using the next link. It also demonstrates how to replicate the memory leaks problem by handling an event.

https://github.com/Drreamer/AttachedPropertyMemoryTest

Dave M · Accepted Answer · 2017-12-31T18:06:08.763

when I close the user control is when the attached property will be recolected because no object is referencing it.

This is false. If you remove the code that unregisters the events, any controls using the attached property will live forever. Why? Because the event handlers you register are static. That means the control will contain a reference to something static preventing the garbage collector from ever collecting it.

The first potential solution to this problem is to use the weak event pattern when registering events. It's for the reason above that I always use the weak event pattern when registering events for my own attached properties.

The annoying thing about this solution is that it requires a rather large amount of boilerplate code. You have to create a new WeakEventManager implementation for every new type of event. Then to receive the weak events, you have to implement an interface (EDIT: unless you are using .NET 4.5 or higher), and that means you can't have a static handler. So then you need class that implements the IWeakEventListner interface, and create and manage instances of that class in your attached property events.

Therefore, the solution I would recommend for you is to actually subclass the DataGrid class and add this functionality as a normal dependency property. If you do it that way, you won't have to register events at all (there are protected methods you can override), and there's no worries about potential memory leaks. The reason I would recommend this solution is because in my experience I have needed to override the DataGrid class for numerous other reasons, many of them could be achieved with attached properties, but a few of them cannot.

The real problem is that the WPF DataGrid implementation is rather half-baked (my personal opinion). There are bugs, default behaviors that I don't like, and incomplete or unimplemented features (such as support for Copy, but not Paste; or the particular issue I think you are trying to solve: a bindable SelectedItems). It's possible to fix all these issues most easily by simply subclassing DataGrid.

If you use the [AddHandler](https://msdn.microsoft.com/en-us/library/hh199430(v=vs.110).aspx) method added in .net 4.5 it removes all of the boilerplate and turns it in to a single line statement to add or remove weak events. You nolonger need to implement IWeakEventListener — Scott Chamberlain, Dec 31 '17 at 15:58
Good point, but you still need to create the WeakEventManager implementation for each event, and the overall point still stands: subclassing DataGrid is much easier. — Dave M, Dec 31 '17 at 18:04
No you don't. `WeakEventManager.AddHandler(miDg, "SelectionChanged", dataGrid_SelectionChanged);` is the only line of code required in .net 4.5 or newer to subscribe using a weak event. You no longer need to create implementations at all as of 4.5. — Scott Chamberlain, Dec 31 '17 at 18:19
This sounds wrong, or? Why would the publisher (the control) stay alive when it has a reference to a static event handler (subscriber)? the event handler is static, so not even an object reference is given to the publisher. But even if, it shouldnt matter. — Welcor, Nov 16 '19 at 18:30

Where to unsubscribe events in an attached property?

2 Answers2