Fill TreeView with data from a collection asynchronously

Question

I am trying to add a batch of nodes to a TreeView asynchronously, in 50ms time intervals, but I am getting an exception that says the collection has been modified. How can I prevent this exception from occurring?

Collection was modified; enumeration operation may not execute.

public partial class Form1 : Form
{

    private SynchronizationContext synchronizationContext;
    private DateTime previousTime = DateTime.Now;
    private List<int> _batch = new List<int>();

    public Form1()
    {
        InitializeComponent();
    }

    private async void button1_Click(object sender, EventArgs e)
    {
        synchronizationContext = SynchronizationContext.Current;

        await Task.Run(() =>
        {
            for (var i = 0; i <= 5000000; i++)
            {
                _batch.Add(i);
                UpdateUI(i);
            }
        });
    }

    public void UpdateUI(int value)
    {
        var timeNow = DateTime.Now;

        if ((DateTime.Now - previousTime).Milliseconds <= 50) return;

        synchronizationContext.Post(new SendOrPostCallback(o =>
        {
            foreach (var item in _batch)
            {
                TreeNode newNode = new TreeNode($"{item}");
                treeView1.Nodes.Add(newNode);
            }


        }), null);

        _batch.Clear();
        previousTime = timeNow;
    }
}

Answer 1

List<T> is not thread safe. You are trying to modify the list while you are trying to read from it in your foreach.

There are a few ways to solve this.

Use ConcurrentQueue

Change:

private List<int> _batch = new List<int>();

To:

var _batch = new ConcurrentQueue<int>();

You can add items by calling:

_batch.Enqueue(1);

Then you can grab them all out and add them in a thread-safe manner:

while(_batch.TryDequeue(out var n))
{
    // ...
}

Use List, but with Thread-synchronization Objects

Create a new object like so:

private readonly _listLock = new object();

Then add these helper methods:

private void AddToBatch(int value)
{
    lock(_listLock)
    {
        _batch.Add(value);
    }
}

private int[] GetAllItemsAndClear()
{
    lock(_listLock)
    {
        try
        {
            return _batch.ToArray();
        }
        finally
        {
            _batch.Clear();
        }
    }
}

This ensures only one thread at a time is operating on the List object.

There are other ways to do this as well, but this is the gist of the problem. Your code won't be as fast because of the overhead of synchronizing data, but you won't crash.