CODEWITHSUNDEEP
c#multithreadinglocking
AndrewR
AndrewR

Reputation: 624

C# Timer and synchrononization primitives

I have a simple code:

public partial class Form1 : Form
{
    Random rng = new Random();
    List<int> list = new List<int>();
    static object lockObject = new object();

    public Form1()
    {
        InitializeComponent();
    }

    private void timer1_Tick(object sender, EventArgs e)
    {
        lock (lockObject)
        {
            foreach (int number in list) //Exception appears here
                if (number > 10)
                    list.Remove(number);
        }
    }

    private void button1_Click(object sender, EventArgs e)
    {
        lock (lockObject)
        {
            list.Add(rng.Next(20));
        }
    }
}

This code doesn't work: sometimes foreach statement throws System.InvalidOperationException: Collection was modified. That means that lock statement doesn't work. I tried replacing it with Semaphore object with no effect.

How can I ensure different segments of code does't modify list at one time? Why lock and Semaphore doesn't work in this code?

UPD: I haven't seen so obvious mistake, sorry. I thought the exception was caused by insertion while foreach, and haven't noticed list.Remove(number);. Thanks for your answers!

Upvotes: 0

Views: 96

Answers (3)

Habib
Habib

Reputation: 223402

The exception doesn't have anything to do with Threads or locking. It is because you are modifying the collection in a foreach loop. Do not do that. Instead use a simple for loop.

Something like:

for (int i = list.Count - 1; i > -1; i--)
{
    if (number > 10)
    {
        if (list[i] == number)
        {
            list.RemoveAt(i);
        }
    }
}

Upvotes: 3

FakeCaleb
FakeCaleb

Reputation: 993

You cannot modify a collection while iterating over it.

So a very simple one liner to do what you want, would be:

list.RemoveAll(x => x > 10);

Upvotes: 3

Mike Corcoran
Mike Corcoran

Reputation: 14564

as was said, the problem is you're calling list.Remove() while enumerating the list. a semi-simple way to fix this without modifying much code is to create a copy of the list when you enumerate it like:

foreach (int number in new List<int>(list))
{
    if (number > 10) 
        list.Remove(number);
}

Upvotes: 2

Related Questions