0

I'm building a WPF application according to MVVM. I have an ObservableCollection in my ViewModel which is being used by a Datagrid. My app needs to check a remote resource for new additions and when these arrive, update the GUI and store the items in a database.

The way I'm doing this is I have a ServiceLayer that checks for new items, saves them to the db, and triggers an event. My UI subscribes to this event and updates the ObservableCollection.

The problem was as soon as new items arrived, the attempt to update the ObservableCollection triggered an error "This type of CollectionView does not support changes to its SourceCollection from a thread different from the Dispatcher thread"

I did some reading and managed to fix this problem by adding a CollectionSynchronization lock

This works - but is it a good approach i.e. are there any obvious issues that it will cause?

In my ServiceLayer

 public async void CheckForNewOffers_Tick(object sender, ElapsedEventArgs e)
    {
        var webOffers = await _webAccess.GetLatestOffers();
        var newOffers = new Collection<Offer>();

        //CheckIfOffersAreNew
        foreach(Offer o in webOffers)
        {
            if(!_currentOffers.Any(co=>co.Id == o.Id))
            {
                _currentOffers.Add(o);
                newOffers.Add(o);
            }
        }

        //Any new offers - save them in DB and signal event
        if(newOffers.Count>0)
        {
            _data.StoreNewOffers(newOffers);
            OnReceivingNewOffers(newOffers);
        }
    }



    public event EventHandler<ICollection<Offer>> ReceivedNewOffers;

    protected virtual void OnReceivingNewOffers(ICollection<Offer> newOffers)
    {
        ReceivedNewOffers?.Invoke(this, newOffers);
    }

}

And in my UI

_itemsLock = new object();
            BindingOperations.EnableCollectionSynchronization(_currentOffers, _itemsLock);

public void HandleNewOffers(object sender, ICollection<Offer> newOffers)
    {
        lock (_itemsLock)
        {
            //Implement code to add newOffers to ObservableCollection
            foreach (Offer no in newOffers)
            {
                if (!_currentOffers.Any(co => co.Id == no.Id))
                {
                    _currentOffers.Add(MapOffer(no));
                }
            }
        }
    }
tonydev314
  • 241
  • 1
  • 2
  • 10
  • Have you considered just using dispatcher.invokeasync to schedule an action calls .add on the observablecollection on the ui thread? – Andy Jul 09 '20 at 12:14
  • My understanding is that using invoke may work but is not a good approach from reading this thread: https://stackoverflow.com/questions/2091988/how-do-i-update-an-observablecollection-via-a-worker-thread – tonydev314 Jul 09 '20 at 12:21
  • If you have such a heavy load of changes that it will matter then you might need another strategy entirely. Personally. I would try the simple dispatcher approach initially. If there are numerous changes per second then next step would be add them to a list, schedule one action per second deals with the new ones. If there are many per second then the ui is likely to be a blur. Showing live changes might not be practical. – Andy Jul 09 '20 at 12:36
  • No - the collection is only updated once every 30 secs by this method, although it is updated occasionally by the GUI. You seem to be suggesting I scrap a working version in favour of a different strategy without really explaining why my strategy is inferior - I accept that the potential problems with using invokeasync may not be an issue in my program, but what issues does my approach cause? – tonydev314 Jul 09 '20 at 12:48

1 Answers1

2

The exception occurs because of thread affinity in WPF, which is by design.

Both the ItemsControl and the CollectionView have affinity to the thread on which the ItemsControl was created, meaning that using them on a different thread is forbidden and throws an exception.

When your collection changes, it will raise a CollectionChanged event, which will be handled by the control that binds the ObservableCollection. This control will then update its items on the same thread. In your example this is not the UI thread, but your worker thread.

Binding operations solution

Your approach using BindingOperations.EnableCollectionSynchronization is available since .NET Framework 4.5 and perfectly valid, but you must ensure that there are no deadlocks with the synchronization mechanism of your choice, which can be tricky. I recommend you to check the out reference for details on that.

Dispatcher solution

A more general way to resolve this issue is to delegate adding items to the UI thread.

public void HandleNewOffers(object sender, ICollection<Offer> newOffers)
{
   // The same as you do, but simplified with Linq to return a filtered enumerable
   var filteredOffers = newOffers.Where(no => !_currentOffers.Any(co => co.Id == no.Id)).Select(MapOffer);

   // Use this to invoke the add method synchronously on the UI thread
   System.Windows.Application.Current.Dispatcher.Invoke(() => AddToCurrentOffers(filteredOffers));

   // Use this to alternatively invoke the add method asynchronously on the UI thread
   System.Windows.Application.Current.Dispatcher.InvokeAsync(() => AddToCurrentOffers(filteredOffers));
}

// Just a helper method that can also be inlined as lambda
private void AddToCurrentOffers(IEnumerable<Offer> offers)
{
   foreach (var offer in offers)
   {
      _currentOffers.Add(offer);
   }
}

What the code does is use the Dispatcher of the application to invoke a helper lambda method that adds your items to the _currentOffers collection on the UI thread. You can either do this synchronously or asynchronously as commented in the code.

I have replaced your loop with a Linq query that results in an enumerable on purpose. This offers the advantage, that you queue the lambda for execution on the UI thread one time for all items, instead of countless times for a single item, which is more efficient to reduce context switches that can degrade performance drastically. On the other hand, adding a large number of items can be a long running operation that blocks the UI thread an therefore freezes your application. In effect, it depends on the actual workload and the frequency of adding items to make the right choice for your use-case. This also applies to the mechanism itself, be it the dispatcher or the binding operations approach.

thatguy
  • 21,059
  • 6
  • 30
  • 40