Gfrktyl

I'm working on a WPF 4.5 desktop application that has several nested collections with the following key aspects:

• writes mainly from UI thread, but also from worker threads


• writes are relatively seldom, mainly directly after a user interaction


• reads from any thread, but esp. one performance critical worker thread with very many reads (iterating in millisecond intervals)


• with the exception of the thread mentioned above, read/write performance should not be critical


• items need to be ordered, i.e. an item's position must always stay the same


• "remove item" must be supported


• "insert at" must be supported, although I'm aware that indices must be handled with care, if several threads are involved


• collection will be used as a WPF binding source and must be observable (implement INotifyCollectionChanged and INotifyPropertyChanged so WPF can update the UI, if items are added/removed)


• 
  collection must support live-shaping (allowing WPF to instantly update a control's sorting/filtering, if relevant items' properties change; requires the underlying collection to implement IList or similar, so a ListCollectionView can be used)


• a lookup via key is not required (or can be achieved using extension methods, e.g. FirstOrDefault)


• approx. max. number of collections < 10k


• approx. max. number of items / collection < 1k

The out-of-the-box system classes have the following issues (for my use-case) which prevent me from using them as-is:

• 
  System.Collections.Concurrent classes do not implement IList - and cannot be used for live-shaping


• 
  System.Collections.ObjectModel.ObservableCollection<T> is not thread-safe

So to fulfill all above requirements I created a wrapper class that implements the required interfaces (e.g. IList, INotifyCollectionChanged...). Internally I chose to use List<T>. (I could have chosen ObservableCollection, but I wanted full control when invoking/dispatching CollectionChanged.)

For all write operations the wrapper class uses lock(_lock) and delegates the call to the inner list. Also - from within the lock - it updates an Array snapshot of the current list, stored in a private field, _snapshot. Then - still from within the lock - it uses System.Windows.Threading.Dispatcher.InvokeAsync() to raise the CollectionChanged event on the correct UI thread.

All read operations use the cached _snapshot, esp. GetEnumerator. The intention behind the snapshot is to avoid locking in the GetEnumerator implementation, for performance reasons of the thread with many reads.

Is the approach ok, what am I missing, what else must I be aware of?

Here's my current code (with some omissions), which appears to work:

EDIT: I included the previously omitted ICollection and IList implementations.

using System;

using System.Collections;

using System.Collections.Generic;

using System.Collections.Specialized;

using System.ComponentModel;

using System.Linq;

using System.Threading;

using System.Windows;



namespace StackOverflow.Questions

{

    public class ObservableConcurrentList<T> : IList, IList<T>, INotifyCollectionChanged, INotifyPropertyChanged

    {

        private readonly System.Windows.Threading.Dispatcher _context;

        private readonly IList<T> _list = new List<T>();

        private readonly object _lock = new object();

        private T _snapshot; 



        public ObservableConcurrentList()

        {

            _context = Application.Current?.Dispatcher;



            updateSnapshot();



            SuppressNotifications = suppressNotifications;

        }



        public event NotifyCollectionChangedEventHandler CollectionChanged;



        public event PropertyChangedEventHandler PropertyChanged;



        private void updateSnapshot()

        {

            lock (_lock) //precautionary; should be re-entry

            {

                Interlocked.Exchange(ref _snapshot, _list.ToArray());

            }

        }



        private void notify(NotifyCollectionChangedEventArgs args)

        {

            if (_context == null)

            {

                invokeCollectionChanged(args);

            }

            else

            {

                _context.InvokeAsync(() => invokeCollectionChanged(args));

            }

        }



        private void invokeCollectionChanged(NotifyCollectionChangedEventArgs args)

        {

            CollectionChanged?.Invoke(this, args);

            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Count)));

        }



        #region IEnumerable

        public IEnumerator<T> GetEnumerator()

        {

            var localSnapshot = _snapshot; //create local variable to protect enumerator, if class member (_snapshot) should be changed/replaced while iterating

            return ((IEnumerable<T>)localSnapshot).GetEnumerator();

        }



        IEnumerator IEnumerable.GetEnumerator()

        {

            return GetEnumerator();

        }

        #endregion



        #region ICollection<T>

        public void Add(T item)

        {

            lock (_lock)

            {

                _list.Add(item);

                updateSnapshot();



                notify(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, _list.Count - 1));

            }

        }



        public bool Contains(T item)

        {

            return _snapshot.Contains(item);

        }



        public void CopyTo(T array, int arrayIndex)

        {

            _snapshot.CopyTo(array, arrayIndex);

        }



        public bool Remove(T item)

        {

            lock (_lock)

            {

                var index = _list.IndexOf(item);

                if (index > -1)

                {

                    if (_list.Remove(item))

                    {

                        updateSnapshot();



                        notify(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index));

                        return true;

                    }

                }



                return false;

            }

        }



        public void Clear()

        {

            lock (_lock)

            {

                _list.Clear();

                updateSnapshot();



                notify(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));

            }

        }



        public bool IsReadOnly => false;



        #endregion



        #region IList<T>



        public int IndexOf(T item)

        {

            return Array.IndexOf(_snapshot, item);

        }



        public void Insert(int index, T item)

        {

            lock (_lock)

            {

                _list.Insert(index, item);

                updateSnapshot();



                notify(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index));

            }

        }



        public void RemoveAt(int index)

        {

            lock (_lock)

            {

                var item = _list[index];

                _list.RemoveAt(index);

                updateSnapshot();



                notify(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index));

            }

        }





        public T this[int index]

        {

            get => _snapshot[index];

            set

            {

                lock (_lock)

                {

                    var item = _list[index];

                    _list[index] = value;

                    updateSnapshot();



                    notify(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, value, item, index));

                }

            }

        }

        #endregion



        #region ICollection (explicit)

        void ICollection.CopyTo(Array array, int index)

        {

            CopyTo((T)array, index);

        }



        public int Count => _snapshot.Length;



        object ICollection.SyncRoot => this; //https://stackoverflow.com/questions/728896/whats-the-use-of-the-syncroot-pattern/728934#728934



        bool ICollection.IsSynchronized => false; //https://stackoverflow.com/questions/728896/whats-the-use-of-the-syncroot-pattern/728934#728934



        #endregion



        #region IList (explicit)



        object IList.this[int index]

        {

            get => ((IList<T>)this)[index];

            set => ((IList<T>)this)[index] = (T)value;

        }



        int IList.Add(object value)

        {

            lock (_lock)

            {

                Add((T)value);



                return _list.Count - 1;

            }

        }



        bool IList.Contains(object value)

        {

            return Contains((T)value);

        }



        int IList.IndexOf(object value)

        {

            return IndexOf((T)value);

        }



        void IList.Insert(int index, object value)

        {

            Insert(index, (T)value);

        }



        bool IList.IsFixedSize => false;



        void IList.Remove(object value)

        {

            Remove((T)value);

        }

        #endregion

    }

}