Observable collections should allow event handlers to see the original items when cleared

[airbreather]

Affects (at least) these two types:

1. System.Collections.ObjectModel.ObservableCollection<T>
2. System.Collections.ObjectModel.ReadOnlyObservableCollection<T>

As a developer who occasionally consumes an instance of either of these two types, I want to write an event handler that sees every element as it's removed from the collection so that I can remove any object references from the element that might cause memory leaks.

Currently, calling Clear() results in the CollectionChanged event getting raised with effectively empty event arguments, at which point the elements are already gone. This can matter if my code adds event handlers to elements of this collection that close over some reference to an object that those elements are supposed to outlive (even if they happen to get removed from the exact collection in question).

Workarounds that I can come up with today:

1. If I own the code that produces all relevant collections, then I can use a subclass of ObservableCollection<T> (along with a subclass of ReadOnlyObservableCollection<T>, if needed) that overrides ClearItems to raise a custom event before invoking the base method.
2. Otherwise, in order to get the behavior I need, I must keep track of the elements of the collection externally, somehow, just for the sake of Reset events.

Scanning through, it looks like this also impacts correctness, regardless of the memory leak concern: we implement a tree, and each node has an observable collection of children, each node also holding a reference to its parent. As we have it implemented, we monitor the children collection for changes and (among other things) we update child-to-parent references whenever the parent's Children collection changes, and we had to use a subclass of ObservableCollection<T> to ensure that we can update the child-to-parent references when a Children collection is cleared.

Proposed alternatives that would make this easier:

1. Somehow find a non-horrible way to populate NotifyCollectionChangedEventArgs.OldItems on a Clear? It feels really bad copying all the items to a brand new array before base.ClearItems() just for the sake of this, though, and guarding it by an opt-in flag feels almost as bad too.
2. Add an event to these two collection types called something like CollectionClearing that gets raised before calling base.ClearItems().
3. Taking the previous item a bit further, maybe we could add INotifyCollectionChanging, analogous to how we have INotifyPropertyChanging as a counterpart to INotifyPropertyChanged?

[grokys]

Please, please. It drives me crazy on a daily basis, that even if I implement my own collection implementing INotifyCollectionChanged I cannot pass this information to NotifyCollectionChangedEventArgs

Even if ObservableCollection couldn't be changed to add this, at least there should be an additional API on NotifyCollectionChangedEventArgs which allows passing the old items with a Reset action.

[eiriktsarpalis]

Somehow find a non-horrible way to populate NotifyCollectionChangedEventArgs.OldItems on a Clear? It feels really bad copying all the items to a brand new array before base.ClearItems() just for the sake of this, though, and guarding it by an opt-in flag feels almost as bad too.

I agree with the sentiment that copying the entire contents of a collection by default is not great. I don't believe all users need to access the collection context when triggering a Reset event, and it might make more sense to simply drain the collection ahead of time or implement a subclass like you already proposed.

Add an event to these two collection types called something like CollectionClearing that gets raised before calling base.ClearItems().

Adding a new NotifyCollectionChangedAction value would be a breaking change to existing event handlers, so we can't do that.

Taking the previous item a bit further, maybe we could add INotifyCollectionChanging, analogous to how we have INotifyPropertyChanging as a counterpart to INotifyPropertyChanged?

I'm not sure what that means, I would recommend filing an API proposal with API diff and examples.

[airbreather]

Taking the previous item a bit further, maybe we could add INotifyCollectionChanging, analogous to how we have INotifyPropertyChanging as a counterpart to INotifyPropertyChanged?

I'm not sure what that means, I would recommend filing an API proposal with API diff and examples.

I'll do something more formal later (hopefully I'll remember), but to try to give a little bit of less-formal clarification:

• Currently, the interface INotifyPropertyChanged has a (perhaps lesser-known) cousin, INotifyPropertyChanging. Its event args provides the name of the property that's about to change, but its contract promises to raise the event before the property's value changes.
• The suggestion here is to add a new interface, INotifyCollectionChanging, that can give subscribers the opportunity to react to changes that are about to happen to a collection, before they happen. Among other things, being able to react to upcoming changes would resolve this issue, because the collection has not yet been cleared when the event is raised.

[ghost]

This issue has been marked with the api-needs-work label. This may suggest that the proposal requires further refinement before it can be considered for API review. Please refer to our API review guidelines for a detailed description of the process.

When ready to submit an amended proposal, please ensure that the original post in this issue has been updated, following the API proposal template and examples as provided in the guidelines.