0

I'm new to C#, but learning it. I figured out C# has many awesome methods such as Parallel.

So let's say my program has this method.

private void stackOverFlowExample()
{
    List<string> testCompany = new List<string>();
    List<People> testList = new List<People>();

    testCompany.Add("Stack Over Flow 1");
    testCompany.Add("Stack Over Flow 2");
    testCompany.Add("Stack Over Flow 3");
    ...(1000 Times)...

    Parallel.ForEach(testCompany, company => 
    {
        testList.Add(new People()
        {
            Name = value1, // Some values
            Address = value2,
            Phone = value3,
            Company = company
        });
    });
}

So does above code have thread related problems? And is there any book or guide related to thread problems solving that you can recommend?

KimchiMan
  • 4,836
  • 6
  • 35
  • 41
  • 4
    There's no locking in your code, so you won't see any deadlocks, but you're accessing a collection that isn't thread safe from multiple threads so behavior is undefined. – Brian Rasmussen Jan 24 '14 at 23:20

3 Answers3

3

That code does have thread related problems, but not the kind that would be called a deadlock.

The problem with the code is that several threads will add items to the list, but the List<T> class is not thread safe, so if any of the threads are adding items at the same time the list object will be corrupted.

As you are not using any synchronising locks in the code, there can be no deadlocks. A deadlock can happen when threads are locking two different things, so even if you added locking in your code to make it thread safe, there could be no deadlocks as you would only lock one thing.

For a simple explanation of deadlocks: How to explain the "deadlock" better?


A thread safe version of your code would use a lock around the code that adds the item to the list, to make sure that only one thread at a time enters that code:

private void stackOverFlowExample()
{
    List<string> testCompany = new List<string>();
    List<People> testList = new List<People>();
    object sync = new Object();

    testCompany.Add("Stack Over Flow 1");
    testCompany.Add("Stack Over Flow 2");
    testCompany.Add("Stack Over Flow 3");
    ...(1000 Times)...

    Parallel.ForEach(testCompany, company => 
    {
        People p = new People()
        {
            Name = value1, // Some values
            Address = value2,
            Phone = value3,
            Company = company
        };
        lock (sync) {
          testList.Add(p);
        }
    });
}
Community
  • 1
  • 1
Guffa
  • 687,336
  • 108
  • 737
  • 1,005
1

No, to get a deadlock you need at least two shared resources to lock. but if you mean a thread safe code, then probably yes, since you access to testList without any control by many threads...

For a deadlock, you have to have something like this

>Thread1
lock(a)
   lock(b)
     DoSomething1()


>Thread2
lock(b)
   lock(a)
     DoSomething2()  
L.B
  • 114,136
  • 19
  • 178
  • 224
0

No, deadlock is connected with locking and not directly with parallel programming. So there could be a deadlock in your code, but it would not be directly connected with the fact that you use Parallel.ForEach.

Example of deadlock:

object locker1 = new object(); 
object locker2 = new object(); 

new Thread (() => { 
    lock (locker1) 
    { 
        Thread.Sleep (1000); 
        lock (locker2); // Deadlock 
    } 
}).Start(); 

lock (locker2) 
{ 
    Thread.Sleep (1000); 
    lock (locker1); // Deadlock 
} 

I recommend you read my latest blog post about the most common mistakes and traps when building multi-threaded applications: http://blog.goyello.com/2014/01/21/threading-in-c-7-things-you-should-always-remember-about/

In case you would like to read more about threading in C#, I recommend you read this free e-book: http://www.albahari.info/threading/threading.pdf

CarenRose
  • 1,266
  • 1
  • 12
  • 24
Paweł Bejger
  • 6,176
  • 21
  • 26