.NET Working with Locking and Threads

Question

Work on this small test application to learn threading/locking. I have the following code, I would think that the line should only write to console once. However it doesn't seem to be working as expected. Any thoughts on why? What I'm trying to do is add this Lot object to a List, then if any other threads try and hit that list, it would block. Am i completely misusing lock here?

class Program
{
    static void Main(string[] args)
    {
        int threadCount = 10;

        //spin up x number of test threads
        Thread[] threads = new Thread[threadCount];
        Work w = new Work();
        for (int i = 0; i < threadCount; i++)
        {
            threads[i] = new Thread(new ThreadStart(w.DoWork));
        }
        for (int i = 0; i < threadCount; i++)
        {
            threads[i].Start();
        }

        // don't let the console close
        Console.ReadLine();
    }
}

public class Work
{
    List<Lot> lots = new List<Lot>();
    private static readonly object thisLock = new object();

    public void DoWork()
    {
        Lot lot = new Lot() { LotID = 1, LotNumber = "100" };
        LockLot(lot);
    }

    private void LockLot(Lot lot)
    {
        // i would think that "Lot has been added" should only print once?
        lock (thisLock)
        {
            if(!lots.Contains(lot))
            {
                lots.Add(lot);
                Console.WriteLine("Lot has been added");
            }
        }
    }
}

Answer 1

The lock statement ensures that two pieces of code will not execute simultaneously.

If two threads enter a lock block at once, the seconbd thread will wait until the first one finishes, then continue and execute the block.

In your code, lots.Contains(lot) is always false because the DoWork method creates a different Lot object in each thread. Therefore, eah thread adds another Lot object after acquiring the lock.

You probably want to override Equals and GetHashCode in your Lot class and make it compare by value, so that lots.Contains(lot) will return true for different Lot objects with the same values.

Answer 2

lock is essentially a critical section, and will only lock the object while the code within is executed. As soon as the code exists the lock block, the object will be unlocked. So... it makes sense that each thread would (eventually) print to console.

You are creating a new Lot object on each thread, so if you have not defined your own Equals method for the object it makes sense that lots.Contains(lot) will always return false.

Answer 3

Why would you only expect it to run once? You call DoWork in 10 different threads, each one creates its own "new Lot()" object. Were you expecting value comparison of Lot objects? Did you override Equals() and implement IEquatable?

Answer 4

You need the lock statement to protect shared data, variables that are read and written by more than one thread at the same time. The "lot" variable doesn't qualify that requirement, every thread creates its own instance of a Lot object. And the reference is stored in a local variable ("lot"), every thread has its own local variables.

The lots field does fit the requirement. There is only one instance of it, because there is only one instance of the Work class, all threads access it. And threads both read and write to the list, respectively through the Contains method and the Add method. Your lock statement prevents a thread from accessing the list at the same time and is correct, Contains can never run at the same time as Add.

You are 95% there, you just missed that each thread has a unique "lot" object. One that cannot have been added to the list before. Every single thread will therefore get a false return from Contains.

If you want the Lot class to have identity, based on the LotID and LotNumber property values instead of just the object instance, then you'll need to give it identity by overriding the Equals() and GetHashCode() method. Check your favorite C# programming book, they all mention this. It doesn't otherwise have anything to do with threading.