Eric after dark
Eric after dark

Reputation: 1808

Problems with OnCollectionChanged and background thread

In my program I use a background worker thread to open files. The main structure of my program is a data bound TreeView. During the file read in process, dynamic TreeView nodes are added to the TreeView as they are read in from the file. These TreeView nodes that I mention are bound to containers called UICollections (a custom class inherited from ObservableCollection<T>). I've created the UICollection<T> class in order to make sure that CollectionViews of this type never have their SourceCollections changed from the background worker thread. I do this by changing a property in a UICollection called IsNotifying to false.

My UICollection<T> class:

public class UICollection<T> : ObservableCollection<T>
{
    public UICollection()
    {
        IsNotifying = true;
    }

    public UICollection(IEnumerable<T> source) 
    {
        this.Load(source); 
    }

    public bool IsNotifying { get; set; }

    protected override void OnPropertyChanged(PropertyChangedEventArgs e)
    {
        if (IsNotifying)
            base.OnPropertyChanged(e);
    }

    //Does not raise unless IsNotifying = true
    protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
    {
        if (IsNotifying)
            base.OnCollectionChanged(e);
    }

    //Used whenever I re-order a collection
    public virtual void Load(IEnumerable<T> items)
    {
        if (items == null)
            throw new ArgumentNullException("items");

        this.IsNotifying = false;

        foreach (var item in items)
            this.Add(item);

        //ERROR created on this line because IsNotifying is always set to true
        this.IsNotifying = true;

        this.Refresh();
    }

    public Action<T> OnSelectedItemChanged { get; set; }

    public Func<T, bool> GetDefaultItem { get; set; }

    public void Refresh()
    {
        OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
    }
}

With that being said, I am having problems implementing this UICollection<T> with my control structure, which involves adding UICollections from a background worker thread.

For clarity, my program moves as follows:

  1. Is a file being opened?: YES -> go into Background worker thread

  2. In background worker thread: Do we need to create new UICollections?: YES -> go to method in UIThread that does so (iterate as needed)

  3. Close thread.

The main concept that needs to be understood is that UICollection.IsNotifying has to be set to false if the background worker thread is open. I have no problem doing this for collections that are already known about, but for the dynamic ones I run into problems.

Sample of what my Background worker thread does:

private void openFile()
{
    //Turn off CollectionChanged for known Collections
    KnownCollections1.IsNotifying = false;
    KnownCollections2.IsNotifying = false; //... and so on

    //Do we need to create new collections? YES -> Call to AddCollection in UIThread

    //Refresh known collections
    App.Current.Dispatcher.BeginInvoke((Action)(() => KnownCollections1.Refresh()));
    App.Current.Dispatcher.BeginInvoke((Action)(() => KnownCollections2.Refresh())); //... and so on
    //If new collections exist find them and refresh them...
}

Method in UIThread that adds collections to TreeView:

public void AddCollection(string DisplayName, int locationValue)
{
     node.Children.Add(CreateLocationNode(displayName, locationValue)); //Add to parent node

     for (int i = 0; i < node.Children.Count(); i++)
     {
        //make sure IsNotifying = false for newly added collection
        if (node.Children[i].locationValue == locationValue) 
            node.Children[i].Children.IsNotifying = false; 
    }

    //Order the collection based on numerical value
    var ordered = node.Children.OrderBy(n => n.TreeView_LocValue).ToList(); 
    node.Children.Clear();
    node.Children.Load(ordered); //Pass to UICollection class -- *RUNS INTO ERROR*
}

With all of that, one of two things will happen... if I comment out the line this.IsNotifying = true;, an exception will blow in OnCollectionChanged because it gets raised while the backround thread is open. If I leave the line as is, the collection will never reflect in the view because OnCollectionChanged never gets raised, notifying the view. What do I need to do to allow the creation of these collections without running into these errors? I'm guessing the problem is either in my AddCollection() function, or in the UICollection<T> class.

Upvotes: 0

Views: 355

Answers (1)

Mike Strobel
Mike Strobel

Reputation: 25623

If I understand you correctly, you are manipulating a collection (or nested collection) on a background thread while the same collection (or a 'parent' collection) is being used as an items source in your UI. This isn't safe, even if you disable change notifications. There are other things, such as user-initiated sorting, expanding a tree node, container recycling due to virtualization, etc., that can cause a collection to be requeried. If that happens while you are updating the collection on another thread, the behavior is undefined. For example, you could trigger a collection to be iterated at the same time an insertion on another thread causes underlying list to be resized, potentially resulting in null or duplicate entries being read. Whenever you share mutable data between two threads, you need to synchronize reads and writes, and since you don't control the WPF internals doing the reading, you can't assume it's safe to do concurrent writing of any kind. That includes modify objects within a UI-bound collection from another thread.

If you need to manipulate a collection on a background thread, take a snapshot of the original, perform whatever modifications you need, then marshal yourself back onto the UI thread to commit the changes (either by replacing the original entirely, or clearing and repopulating the collection). I use this technique to safely perform background sorting, grouping, and filtering on grid views with large data sets. But if you do this, be careful to avoid modifying items contained within the collection, as they may still be referenced by your UI. It may also be necessary to detect any changes that occur on the UI thread which may invalidate your background updates, in which case you will need to discard your changes when you marshal yourself back to the UI thread, take another snapshot, and begin again (or come up with a more elaborate way to reconcile the two sets of changes).

Upvotes: 2

Related Questions