A 'ThreadSafe' Observable Collection - C#

Question

Seeing as a concurrent collection does not exist in .net that allows specific items to be removed, I have put together the following class.

It's important to note that it is only threadsafe until the LockTimeout on a given operation expires.

The main goal was to to guard against cheeky 'InvalidOperationException: Collection Was Modified' exceptions, which occur if I'm enumerating in one thread, and adding / removing in another.

I've defaulted the LockTimeout to 10 seconds, but in reality, 1 second would still be plenty (in my user case at least).

Lastly, this specific implementation also incorporates INotifyCollectionChanged and INotifyPropertyChanged.

public class ThreadsafeObservableCollection<T> : IList<T>, INotifyCollectionChanged, INotifyPropertyChanged
{
    public event NotifyCollectionChangedEventHandler CollectionChanged;

    public event PropertyChangedEventHandler PropertyChanged;

    private readonly ConcurrentQueue<PropertyChangedEventArgs> _propertyChangedEvents = new ConcurrentQueue<PropertyChangedEventArgs>();

    private readonly ConcurrentQueue<NotifyCollectionChangedEventArgs> _collectionChangedEvents = new ConcurrentQueue<NotifyCollectionChangedEventArgs>();

    private readonly ObservableCollection<T> _collection;

    private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1);

    private static TimeSpan LOCK_TIMEOUT = TimeSpan.FromSeconds(10);

    public ThreadsafeObservableCollection()
    {
        _collection = new ObservableCollection<T>();
        _collection.CollectionChanged += _collection_CollectionChanged;
        (_collection as INotifyPropertyChanged).PropertyChanged += _collection_PropertyChanged;
    }

    private void Wait()
    {
        while (!_semaphore.Wait(LOCK_TIMEOUT))
            _semaphore.Release();
    }

    public void Add(T item)
    {
        Wait();
        try
        {
            _collection.Add(item);
        }
        finally
        {
            _semaphore.Release();
        }
        FireOutstandingEvents();
    }

    public void Clear()
    {
        Wait();
        try
        {
            _collection.Clear();
        }
        finally
        {
            _semaphore.Release();
        }
        FireOutstandingEvents();
    }

    public bool Contains(T item)
    {
        Wait();
        bool result;
        try
        {
            result = _collection.Contains(item);
        }
        finally
        {
            _semaphore.Release();
        }
        FireOutstandingEvents();
        return result;
    }

    public int Count
    {
        get
        {
            Wait();
            int count;
            try
            {
                count = _collection.Count;
            }
            finally
            {
                _semaphore.Release();
            }
            FireOutstandingEvents();
            return count;
        }
    }

    public bool IsReadOnly => false;

    public T this[int index]
    {
        get
        {
            Wait();
            T item;
            try
            {
                item = _collection[index];
            }
            finally
            {
                _semaphore.Release();
            }
            FireOutstandingEvents();
            return item;
        }
        set
        {
            Wait();
            try
            {
                _collection[index] = value;
            }
            finally
            {
                _semaphore.Release();
            }
            FireOutstandingEvents();
        }
    }

    public void CopyTo(T[] array, int arrayIndex)
    {
        Wait();
        try
        {
            _collection.CopyTo(array, arrayIndex);
        }
        finally
        {
            _semaphore.Release();
        }
        FireOutstandingEvents();
    }

    public int IndexOf(T item)
    {
        Wait();
        int index;
        try
        {
            index = _collection.IndexOf(item);
        }
        finally
        {
            _semaphore.Release();
        }
        FireOutstandingEvents();
        return index;
    }

    public void Insert(int index, T item)
    {
        Wait();
        try
        {
            _collection.Insert(index, item);
        }
        finally
        {
            _semaphore.Release();
        }
        FireOutstandingEvents();
    }

    public bool Remove(T item)
    {
        Wait();
        bool result;
        try
        {
            result = _collection.Remove(item);
        }
        finally
        {
            _semaphore.Release();
        }
        FireOutstandingEvents();
        return result;
    }

    public void RemoveAt(int index)
    {
        Wait();
        try
        {
            _collection.RemoveAt(index);
        }
        finally
        {
            _semaphore.Release();
        }
        FireOutstandingEvents();
    }

    private void FireOutstandingEvents()
    {
        while (_propertyChangedEvents.TryDequeue(out PropertyChangedEventArgs arg))
            PropertyChanged?.Invoke(this, arg);
        while (_collectionChangedEvents.TryDequeue(out NotifyCollectionChangedEventArgs arg))
            CollectionChanged?.Invoke(this, arg);
    }

    private void _collection_PropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        _propertyChangedEvents.Enqueue(e);
    }

    private void _collection_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        _collectionChangedEvents.Enqueue(e);
    }

    public IEnumerator<T> GetEnumerator()
    {
        Wait();
        try
        {
            return ((IEnumerable<T>)_collection.ToArray()).GetEnumerator();
        }
        finally
        {
            _semaphore.Release();
        }
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        Wait();
        try
        {
            return _collection.ToArray().GetEnumerator();
        }
        finally
        {
            _semaphore.Release();
        }
    }
}

Answer 1

The Wait method is not correct. If the timeout expires the semaphore is released without being taken. This may allow code to run immediately afterwards because there's a slot available (not really as it was already taken but erroneously released) but in the future when Release is called it will fail because the semaphore has already reached its maximum size.

private void Wait()
{
    while (!_semaphore.Wait(LOCK_TIMEOUT))
        _semaphore.Release();
}

Every successful call to Wait must be paired with a call to Release. A Wait returning false is not successful.