7

Hei,

In my application i'm using DataGrid to show some data. To get everything working with threading i'm using AsyncObservableCollection as DataContext of DataGrid. When my application starts it looks for files in some folders and updates AsyncObservableCollection. Finding files is done on a separate thread:

Task.Factory.StartNew(() => _cardType.InitAllOrdersCollection())
    .ContinueWith((t) => ThrowEvent(), TaskContinuationOptions.None);

Where all the loading logic is in InitAllOrdersCollection() method.

Now here's where things go bad, when i start the application for some reason i get 2 rows with same data in DataGrid even if there is one item in collection and only one file in folders. If i add a delay(Thread.Sleep() 50ms minimum) before loading files then DataGrid show everything correctly (no extra row). Delay has to be added to the Thread what is loading the files (The one created with Task.Factory.StartNew()).

Have anybody encountered something similar or is there something else i should try? Thanks in advance!

EDIT: Adding some code as requested:

public AsyncObservableCollection<IGridItem> OrdersCollection = new AsyncObservableCollection<IGridItem>();

public void InitAllOrdersCollection()
{
    // Thread.Sleep(50); <-- this sleep here fixes the problem!
    foreach (var convention in FileNameConventions)
    {
        var namePatterns = convention.NameConvention.Split(',');
        foreach (var pattern in namePatterns)
        {
            var validFiles = CardTypeExtensions.GetFiles(this.InputFolder, pattern, convention);
            if (validFiles.Any())
            {
                this.FilesToOrders(validFiles, convention);
            }
        }
    }
}

public static List<string> GetFiles(string inputFolder, string pattern, FileNameConvention convention)
{
    var files = Directory.GetFiles(inputFolder, pattern);
    return files.Where(file => IsCorrect(file, convention)).AsParallel().ToList();
}

// Adds new order to OrdersCollection if its not there already!
private void FilesToOrders(List<string> dirFiles, FileNameConvention convention)
{
    foreach (var dirFile in dirFiles.AsParallel())
    {
        var order = new Order(dirFile, this, convention);

        if (!this.OrdersCollection.ContainsOrder(order))
        {
                this.OrdersCollection.Add(order);
        }
    }
}

public static bool ContainsOrder(this ObservableCollection<IGridItem> collection, Order order)
{
    return collection.Cast<Order>().Any(c=>c.Filepath == order.Filepath);
}

FilesToOrders() method is the one what adds the new orders to the AsyncObservableCollection. Hope this helps.

hs2d
  • 6,027
  • 24
  • 64
  • 103
  • Could you please add some detail on the InitAllOrdersCollection method? I have created a simple test program and I am not getting any duplicates. Perhaps there is something in you code that I am not doing to replicate this kind of behaviour. – Justin Bannister Dec 03 '12 at 14:59
  • I doubt the problem has to do with the Data Grid control. Check what is in `OrdersCollection` - your logic might be putting some "blank models" in the collection. Also, why do you iterate through `dirFiles.AsParallel()` instead of just `dirFiles`? Is there any time-consuming operation in that loop? – default.kramer Dec 03 '12 at 15:27
  • OrderCollection contains only the right amount of items, i have checked that several times, but still they are showed double in DataGrid. Here's some more info about [AsParallel()](http://www.albahari.com/threading/part5.aspx#_PLINQ). Ok, maybe calling AsParallel() in those places isnt doing anything, but thats not the problem here.. – hs2d Dec 03 '12 at 15:32
  • I'm just guessing as I can't easily reconstruct it here, but this looks like the initialization of the datagrid expects initially an empty list. The additional 50ms allow the DataGrid to finish initialization - something along those lines. – StampedeXV Dec 03 '12 at 16:38
  • Where are you firing the Property Changed event to tell the DataGrid that the collection has changed? – Justin Bannister Dec 03 '12 at 16:56
  • @JustinBannister: ObservableCollection executes those events automatically when its binded as ItemSource to DataGrid. – hs2d Dec 03 '12 at 22:26

2 Answers2

5

Add CanUserAddRows="False" to your XAML file

<DataGrid CanUserAddRows="False"../>
Undo
  • 25,519
  • 37
  • 106
  • 129
Lana
  • 51
  • 1
  • 1
3

Maybe I'm missing something obvious, but the AsyncObservableCollection implementation in the link you posted doesn't look thread-safe to me.

I can see it includes code to fire the CollectionChanged / PropertyChanged events on the creator (consumer) thread, but I don't see any synchronization to make access to the items in the collection thread-safe.

UPDATE

As far as I can see you can have the following happening concurrently, without any synchronization:

  • the worker (producer) thread is inserting item(s)

  • the UI (consumer) thread is enumerating items

One possibility might be to modify AsyncObservableCollection.InsertItem to call SynchronizationContext.Send to insert the item on the consumer thread, though this will of course have an effect on performance (producer waits for consumer thread to complete insertion before continuing).

An alternative approach would be to use a standard ObservableCollection that is only ever accessed on the consumer thread, and use SynchronizationContext.Post to post items to insert from the producer thread. Something like:

foreach (var dirFile in dirFiles.AsParallel())
{
    var order = new Order(dirFile, this, convention);

    _synchronizationContext.Post(AddItem, order);

}

...

void AddItem(object item)
{
    // this is executed on the consumer thread
    // All access to OrderCollection on this thread so no need for synchnonization
    Order order = (Order) item;
    if (!OrdersCollection.ContainsOrder(order))
    {
        OrdersCollection.Add(order);
    }
}
Joe
  • 122,218
  • 32
  • 205
  • 338
  • Look at the updated solution in the bottom of that blog post. Its using `SynchronizationContext` to post the PropertyChanged event to the collection thread. – hs2d Dec 03 '12 at 22:22
  • @hs2d - yes I looked at that. I don't see a problem with the event firing, but I do see a potential problem with accessing the items in the collection. – Joe Dec 04 '12 at 08:28
  • Hmm, that looks interesting and when i tried this it actually worked. Need to make some more tests but i think we have a solution. – hs2d Dec 04 '12 at 09:30
  • @hs2d I have the same problem with AsyncObservableCollection. Did you ditch that entirely? Could you elaborate on your final solution to this issue? – Werner May 22 '18 at 09:36