CODEWITHSUNDEEP
c#multithreading
user184805
user184805

Reputation: 1869

Thread lock clarification

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?

Upvotes: 0

Views: 185

Answers (5)

Hoy Cheung
Hoy Cheung

Reputation: 1670

I have changed the code, which proves lock does nothing. I expected add2 won't show up until add1 has finished. But add1 and add2 just mingled together. using System; using System.Threading;

public static class Example {

public static void Main()
{
    int data= 0;

    Thread t1 = new Thread(()=> add1(ref data));
    Thread t2 = new Thread(() => add2(ref data));
    t1.Start();
    t2.Start();
}

static void add1(ref int x)
{
    object lockthis = new object();
    lock (lockthis)
    {
        for (int i = 0; i < 30; i++)
        {
            x += 1;
            Console.WriteLine("add1 " + x);

        }
    }
}
static void add2(ref int x)
{
    object lockthis = new object();
    lock (lockthis)
    {
        for (int i = 0; i < 30; i++)
        {
            x += 3;
            Console.WriteLine("add2 " + x);

        }
    }
}

}

Upvotes: 0

Frederik Gheysels
Frederik Gheysels

Reputation: 56934

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);
        }

Upvotes: 1

John Fisher
John Fisher

Reputation: 22721

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.

Upvotes: 1

John Gietzen
John Gietzen

Reputation: 49534

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.

Upvotes: 3

Anton Gogolev
Anton Gogolev

Reputation: 115691

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.

Upvotes: 1

Related Questions