ObservableCollection not thread-safe even in .NET 4.5?

Question

I am banging my head against the virtual wall for days now. The BindingOperations.EnableSynchronization method seems to work only partial in .NET 4.5.

I wrote a test that fails sometimes:

        object blah = new object();

        Application app = Application.Current == null ? new Application() : Application.Current;
        SynchronizationContext.SetSynchronizationContext(new SynchronizationContext());
        ObservableCollection<ThreadSafeObservableTestObject> collection = null;
        collection = new ObservableCollection<ThreadSafeObservableTestObject>();

        BindingOperations.EnableCollectionSynchronization(collection, blah);

        CollectionTestWindow w = new CollectionTestWindow();

        Task.Factory.StartNew(() =>
        {
            Thread.Sleep(2000);
            w.TestCollection = collection;
            collection.CollectionChanged += collection_CollectionChanged;
            collection.Add(new ThreadSafeObservableTestObject() { ID = 1, Name = "Sandra Bullock" });
            collection.Add(new ThreadSafeObservableTestObject() { ID = 2, Name = "Jennifer Aniston" });
            collection.Add(new ThreadSafeObservableTestObject() { ID = 3, Name = "Jennifer Lopez" });
            collection.Add(new ThreadSafeObservableTestObject() { ID = 4, Name = "Angelina Jolie" });
            collection.Add(new ThreadSafeObservableTestObject() { ID = 5, Name = "Mary Elizabeth Mastrantonio" });
            Thread.Sleep(5000);
            System.Windows.Application.Current.Dispatcher.Invoke(() => w.Close());
            System.Windows.Application.Current.Dispatcher.Invoke(() => Application.Current.Shutdown());
        });
        app.Run(w);

The TestCollectionWindow looks like this:

    <ItemsControl ItemsSource="{Binding TestCollection}" Name="list">
        <ItemsControl.ItemTemplate>
            <DataTemplate>
                <StackPanel Orientation="Horizontal">
                    <TextBlock Text="{Binding Name}" />
                    <TextBlock Text="{Binding ID}" />
                </StackPanel>
            </DataTemplate>
        </ItemsControl.ItemTemplate>
    </ItemsControl>

So nothing magic here. But the result is almost every time that some entries are twice in the UI - the same objects! The result window looks like this then:

Sandra Bullock 1
Jennifer Aniston 2
Jennifer Lopez 3
Angelina Jolie 4
Mary Elizabeth Mastrantonio 5
Jennifer Aniston 2

As you can clearly see Jennifer Aniston is listed twice. This can be reproduced easily. Is this a general problem or is there anything wrong with this test, such as a flawed application instantiation?

Thank you in advance!

Solution 1

The class is documented to not be thread-safe:

Thread Safety
Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

So no, it is not thread-safe.

Note that BindingOperations.EnableCollectionSynchronization does not magically make the entire collection thread-safe. It only tells the binding system which locking object that you intend to use in order to prevent multiple threads accessing the collection at the same time.

Since you're not actually using the locking object, you might as well not call that method, the results will be equally unpredictable.

Try issuing a lock on the blah object around each statement that accesses the collection. Unfortunately the details around data binding in WPF is unknown to me, so I have no idea if that is enough.

Solution 2

I recently needed to solve this issue as well and wrote about my solution on CodeProject: http://www.codeproject.com/Tips/998619/Thread-Safe-ObservableCollection-T

The solution involved using a SyncronizationContext to invoke the event handlers on the UI thread and a ReaderWriterLockSlim to ensure only one write occurred at a time and that no writes occurred during a read.

Full source code is available at the CodeProject link above but here's some snippets:

public SynchronizedObservableCollection()
{
    _context = SynchronizationContext.Current;
}

private void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
{
    var collectionChanged = CollectionChanged;
    if (collectionChanged == null)
    {
        return;
    }

    using (BlockReentrancy())
    {
        _context.Send(state => collectionChanged(this, e), null);
    }
}

public bool Contains(T item)
{
    _itemsLock.EnterReadLock();

    try
    {
        return _items.Contains(item);
    }
    finally
    {
        _itemsLock.ExitReadLock();
    }
}

public void Add(T item)
{
    _itemsLock.EnterWriteLock();

    var index = _items.Count;

    try
    {
        CheckIsReadOnly();
        CheckReentrancy();

        _items.Insert(index, item);
    }
    finally
    {
        _itemsLock.ExitWriteLock();
    }

    OnPropertyChanged("Count");
    OnPropertyChanged("Item[]");
    OnCollectionChanged(NotifyCollectionChangedAction.Add, item, index);
}

Solution 3

Since you're using .Net 4.5, you can use the Thread-safe collections, ConcurrentDictionary, ConcurrentBag etc, whichever suits your needs : http://msdn.microsoft.com/en-us/library/dd997305.aspx

This article may also help : http://www.codeproject.com/Articles/208361/Concurrent-Observable-Collection-Dictionary-and-So