0

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.

Eric after dark
  • 1,768
  • 4
  • 31
  • 79

1 Answers1

2

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).

Mike Strobel
  • 25,075
  • 57
  • 69
  • Hi Mike. Sounds like you understand my post quite well. I know it was long, but I was trying to be as clear as possible. Are you recommending that I maybe freeze the background thread whenever I move to the UIThread to make changes to my collections? – Eric after dark Nov 07 '14 at 19:23
  • I am saying that you cannot modify any collection on a background thread if it is referenced by your UI thread. The best you can generally do is make a copy of the original collection, make your background modifications on the copy, and when finished, marshal yourself back onto the UI thread to replace the original collection with the modified copy. If you end up doing modifications on both the UI thread and a background thread, then things get more complicated, and you will need to come up with a way to reconcile both sets of changes, but this will need to be done on the UI thread as well. – Mike Strobel Nov 07 '14 at 19:27
  • Thanks @MikeStrobel. Would it be easier to queue all of the collections that need to be added and add them after the background worker thread is closed? – Eric after dark Nov 07 '14 at 21:00
  • @Ericafterdark Could be. If theses collections are wholly new additions that you are populating on a background thread, then yes, it may be advantageous to marshal them back to the UI thread to insert them into the tree, either all at once, or as they become completely populated. The important thing is that the UI thread can't read from the collection until it has been completely populated/modified and "forgotten" by the background thread. – Mike Strobel Nov 07 '14 at 21:19