How do you safely enumerate a List<Weakreference> without finalizers getting in the way?

Question

I have a static list of WeakReference's in my application. At some point, I want to take a snapshot of all the currently "alive" objects in this list.

Code is like this:

private static readonly List<WeakReference> myObjects = new List<WeakReference>();

public static MyObject[] CollectObjects()
{
    var list = new List<MyObject>();
    foreach (var item in myObjects)
    {
        if (!item.IsAlive)
            continue;
        var obj = item.Target as MyObject;
        list.Add(obj);
    }
    return list.ToArray();
}

The problem I'm having is that I sometimes (rarely) get a "Collection Was Modified" exception in the foreach loop above. I only add/remove from this list in the MyObject constructor/finalizers, which looks like this:

public class MyObject
{
    private static readonly object _lockObject = new object();
    WeakReference referenceToThis;
    public MyObject()
    {
        lock (_lockObject)
        {
            referenceToThis = new WeakReference(this);
            myObjects.Add(referenceToThis);
        }
    }
    ~MyObject()
    {
        lock (_lockObject)
        {
            myObjects.Remove(referenceToThis);
        }
    }
}

Since nothing else in my code is touching the list, my assumption is therefore that the garbage collector is finalizing some of those objects just as I try to enumerate the list.

I thought about adding a lock (_lockObject) around the foreach loop but I'm not sure how such a lock would affect the GC?

Is there a better/correct/safer way to enumerate over a List of WeakReferences?

Answer 1

A finalizer introduces a significant overhead in the entire garbage collection mechanism, so it is best to avoid it if you can. And in your case, you can avoid it and greatly simplify your design.

Instead of having your objects detecting their own finalization via finalizers and removing themselves from that list, replace your List<WeakReference<T>> with a WeakCollection<T>.

WeakCollection<T> is a collection, not a list. Never use a list there where a collection would suffice.

WeakCollection<T> fully encapsulates the fact that it contains weak references, meaning that you use it as a regular list, there is nothing about weak references in its interface.

WeakCollection<T> automatically removes weak references from itself when it detects that they have expired. (You will find that implementing it as a list is significantly more complicated than implementing it as a collection, so once again -- never use a list there where a collection would suffice.)

Here is a sample implementation of a WeakCollection<T>. Beware: I have not tested it.

namespace Whatever
{
    using Sys = System;
    using Legacy = System.Collections;
    using Collections = System.Collections.Generic;

    public class WeakCollection<T> : Collections.ICollection<T> where T : class
    {
        private readonly Collections.List<Sys.WeakReference<T>> list = new Collections.List<Sys.WeakReference<T>>();

        public void                           Add( T item )   => list.Add( new Sys.WeakReference<T>( item ) );
        public void                           Clear()         => list.Clear();
        public int                            Count           => list.Count;
        public bool                           IsReadOnly      => false;
        Legacy.IEnumerator Legacy.IEnumerable.GetEnumerator() => GetEnumerator();

        public bool Contains( T item )
        {
            foreach( var element in this )
                if( Equals( element, item ) )
                    return true;
            return false;
        }

        public void CopyTo( T[] array, int arrayIndex )
        {
            foreach( var element in this )
                array[arrayIndex++] = element;
        }

        public bool Remove( T item )
        {
            for( int i = 0; i < list.Count; i++ )
            {
                if( !list[i].TryGetTarget( out T target ) )
                    continue;
                if( Equals( target, item ) )
                {
                    list.RemoveAt( i );
                    return true;
                }
            }
            return false;
        }

        public Collections.IEnumerator<T> GetEnumerator()
        {
            for( int i = list.Count - 1; i >= 0; i-- )
            {
                if( !list[i].TryGetTarget( out T element ) )
                {
                    list.RemoveAt( i );
                    continue;
                }
                yield return element;
            }
        }
    }
}