views:

63

answers:

4

I am a beginer in programing.

When i execute the code by locking the operation:

class ThreadSafe
{
    static List<string> list = new List<string>();
    static object obj=new object();
    static void Main()
    {
        new Thread(AddItems).Start();
        new Thread(AddItems).Start();

        foreach (string str in list)
        {
            Console.WriteLine(str);
        }
        Console.WriteLine("Count=" + list.Count.ToString());
        Console.ReadKey(true);
    }
    static void AddItems()
    {
        lock (obj)
        {
            for (int i = 1; i < 10; i++)
             list.Add("Item " + i.ToString());
        }

    }
}

even i am reciving,"InvalidOperationException".What would be the code alteration?

+1  A: 

You're enumerating over a collection with foreach (string str in list) while modifying it in AddItems(). For this code to work property you'll either have to Thread.Join() both threads (so that both will finish adding items to a list; I'm not sure, however if Add is threadsafe; I bet it's not, so you'll have to account for that by locking on SyncRoot) or use a ReaderWriterLock to logically separate these operations.

Anton Gogolev
+2  A: 

The issue is that your threads are altering the list while it is trying to be read.

class ThreadSafe
{
    static List<string> list = new List<string>();
    static object obj=new object();
    static void Main()
    {
        var t1 = new Thread(AddItems);
        var t2 = new Thread(AddItems);

        t1.Start();
        t2.Start();

        t1.Join();
        t2.Join();

        foreach (string str in list)
        {
            Console.WriteLine(str);
        }
        Console.WriteLine("Count=" + list.Count.ToString());
        Console.ReadKey(true);
    }
    static void AddItems()
    {
        for (int i = 1; i < 10; i++)
            lock (obj)
            {
                list.Add("Item " + i.ToString());
            }
    }
}

The difference being that this code waits for both threads to complete before showing the results.

I also moved the lock around the specific instruction that needs to be locked, so that both threads can run concurrently.

John Gietzen
+1  A: 

You are looping through the result list before the two AddItems threads have finished populating the list. So, the foreach complains that the list was updated while it was looping through that list.

Something like this should help:

System.Threading.Thread.Sleep(0); // Let the other threads get started on the list.
lock(obj) 
{
  foreach (string str in list)
  {
    Console.WriteLine(str);
  }
}

Watch out though! This doesn't guarantee that the second thread will finish it's job before you have read through the list provided by the first thread (assuming the first thread grabs the lock first).

You will need some other mechanism (like John Gietzen's solution) to know when both threads have finished, before reading the results.

John Fisher
Why the -1? He is seeking to understand the problem, and this helps to explain it.
John Fisher
+1 agreed with John Fisher
Michael Damatov
+1  A: 

Use the debugger. :)

You receive the InvalidOperationException on the foreach. What happens, is that the foreach is executed while your threads are still running. Therefore, you're iterating over your list, while items are being added to the list. So, the contents of the list are changing, and therefore, the foreach throws an exception.

You can avoid this problem by calling 'Join'.

        static void Main()
        {
            Thread t1 = new Thread (AddItems);
            Thread t2 =new Thread (AddItems);

            t1.Start (); 
            t2.Start ();

            t1.Join ();
            t2.Join ();

            foreach( string str in list )
            {
                Console.WriteLine (str);
            }
            Console.WriteLine ("Count=" + list.Count.ToString ());
            Console.ReadKey (true);
        }
Frederik Gheysels
3 minutes late. ;) That's almost an exact duplicate of mine.
John Gietzen
Yeah, I've upgraded your answer since you were first. (You must have posted it while I was still running the example in vs.net).
Frederik Gheysels