Does ListCollectionView leak memory?

Question

I've been investigating how to avoid memory leaks caused by strong references to the INotifyCollectionChanged event from a view model. I was playing around with using a ListCollectionView to see if that would deal with it for me. I think that the following is leaking memory, am I doing something wrong?

var stuff = new ObservableCollection<string>();
while (true)
{
    var result = new ListCollectionView(stuff);
    // Just to keep make sure that the memory I'm seeing 
    // isn't waiting to be GC'd
    GC.Collect(); 
}

Ferd

Possible duplicate of #28440365 and also check this: eidias.com/blog/2014/2/24/wpf-collectionview-can-leak-memory – Dyche

Answer 1

The documentation for ListCollectionView is not great but if you noticed there is a method DetachFromSourceCollection. The remarks for this call mention unsubscribing and allowing garbage collection.

    var stuff = new ObservableCollection<string>();
    while (true)
    {
        ListCollectionView result = new ListCollectionView(stuff);

        //Use this method to unsubscribe to events on the underlying collection and allow the CollectionView to be garbage collected.
        result.DetachFromSourceCollection();
        //When finished set to null
        result = null;
        GC.Collect();
    }

Escalante

@Geoff, this should be the answer you seek. DetachFromSourceCollection is the thing your ViewModel should call. If you don't create ListCollectionView, then you don't have to do anything (e.g. when binding directly to ObservableCollection). You should only worry if your collection doesn't implement INotifyPropertyChanged (because it will leak). – Cyclohexane

Just to confirm, the reference source for DetachFromSourceCollection shows event handlers in the _sourceCollection as INotifyCollectionChanged; getting removed. – Petterson

Answer 2

I initially posted this as a comment, but I think it makes a better answer, so ...

a) if you're sure you've found a problem with the .NET framework, you're probably doing something wrong. It's not impossible, it's just not likely. b) that GC.Collect() isn't going to do what you're thinking it will.

I think you need to review how GC.Collect() works.

---

MSDN GC.Collect Method

Remarks

Use this method to try to reclaim all memory that is inaccessible.

All objects, regardless of how long they have been in memory, are considered for collection; however, objects that are referenced in managed code are not collected. Use this method to force the system to try to reclaim the maximum amount of available memory.

---

For starters, you don't show us where you're disposing of that memory that the ListCollectionView(stuff). You're just allocating new and allocating new, but you never dispose of the old. So yeah, it's going to leak like crazy. Until the GC runs and tries to collect.

If you do the same thing you demonstrate here with a list of strings it will most likely do the same thing. But for what you've shown, I expect it to leak.

Concomitant

Thanks for the answer! So how would you dispose the ListCollectionViews? – Ferd

@Geoff: just -- not! Garbage is called garbage collection for a reason. Why would you dispose it? – Grochow

@Ferd it's trying to collect. But it can't for some reason. There are rules to GC collection, such as age of the object. If you just created it, and you didn't dispose of it, it won't just let it go. It assumes an age in the GC collector. – Concomitant

Answer 3

when you call GC.Collect, you variable result is still in scope so it won't be collected since there is one pointer to the data. anyway even if it wasn't the case. what garbage collection does is non deterministic as far as application code is concerned. like drachenstern said it will only try! and it will succeed eventually but you can't be sure when!

Handle

I guess I should explain that I wasn't expecting GC.Collect to collect the var created in the same iteration, but the one from the previous. Basically if I don't GC my laptop freezes after a few mins, with the GC.Collect I get a slow and steady increase. – Ferd

yes on each iteration it should collect the object from last iteration but not every time since garbage collection works on pages of memory and is not affected by the order of allocation. what happens in gc is a black art! you should always handle it manually for any memory intensive operation. let garbage collection handle leaked memory only. – Handle

this might be interesting geekswithblogs.net/NewThingsILearned/archive/2008/02/07/… – Handle

Answer 4

The CollectionView holds a reference to the source collection’s CollectionChanged event - hence GC can not collect the view until the source collection is disposed off and collected.

This is also clear from the documentation of CollectionView

    /// <summary>
    /// Detach from the source collection.  (I.e. stop listening to the collection's
    /// events, or anything else that makes the CollectionView ineligible for
    /// garbage collection.)
    /// </summary>
    public virtual void DetachFromSourceCollection()

This blog describes your issue and suggest two possible solutions:
http://www.eidias.com/blog/2014/2/24/wpf-collectionview-can-leak-memory ...

Kristenkristi

Answer 5

With each iteration result is being reassigned so that there won't be a reference to the ListCollectionView from the previous iteration. But calling GC.Collect only schedules those items to have their memory reclaimed when the CLR decides to do the actual garbage collection. If you would like to see the memory getting reclaimed sooner, try adding GC.WaitForPendingFinalizers(); immediately after your call to GC.Collect();.

Parenteau

Answer 6

The best way to do this is to use Scopes / Anonymous functions. Lambada is grate for that

var stuff = new ObservableCollection<string>();
ClosureDelegate closure = (x) => {
    ListCollectionView result = new ListCollectionView(x);
    //Use this method to unsubscribe to events on the underlying collection and allow the CollectionView to be garbage collected.
    result.DetachFromSourceCollection();
};

while (true)
{
    closure(stuff);
    GC.Collect(); 
}

Using this method your throwing result out of scope as its method has been removed and this would use as little memory as possible.

Taken from: https://msdn.microsoft.com/en-gb/library/bb882516.aspx?f=255&MSPPError=-2147217396

Racialism