CODEWITHSUNDEEP
c#multithreadinglockingtask
Zeus
Zeus

Reputation: 3357

Is there a deadlock case for Multiple locks on same object?

We have a code with two locks locking on same object in different methods. Both methods can be called from different threads.

Can this go into deadlock scenario ? Should we use same lock1 in both methods ?

Reason for using two different locks is that removal happens from various tasks running in parallel but updating the list happens from new thread running every few seconds.

private static Object Lock1 = new Object();
private static Object Lock2 = new Object();
private static List<string> list = new List<string>();

public void Method1()
{
  lock(Lock1)
  {
   //update list
  }
}

public void Method2()
{
  lock(Lock2)
  {
    //remove from list
  }   
}

Update

Thanks for healthy discussion, I have updated my code to use thread safe collection BlockingCollection provided by .net. I continue to use the lock1 as I need to control how many number of objects the list can contain. I have removed the lock2 now as its not needed with the thread safe list.

Upvotes: 2

Views: 2184

Answers (2)

Eric Lippert
Eric Lippert

Reputation: 660030

Can this go into deadlock scenario?

No.

Should we use same lock1 in both methods ?

Yes. You should always lock on the same lock object when accessing a particular object on multiple threads. In your specific case the code you've shown is completely wrong and broken. You should have one lock per list, and consistently take out that lock every time you access the list.

If you are updating the list variable then same thing -- you should have one lock for the list variable, and every single time you access the variable for any reason needs to be under that lock.

Reason for using two different locks is that removal happens from various tasks running in parallel but updating the list happens from new thread running every few seconds.

That doesn't matter. All updates, whether they are removals or otherwise, must happen under the same lock.

If you are in a situation where you have many frequent readers and few writers, there are special-purpose locks for those cases. But that is not your scenario.

Some questions you did not ask:

What causes a deadlock?

void Transfer(Account a1, Account a2, decimal amount)
{
  lock(a1)
  {
    lock(a2)
    {
      // do the transfer
    }
  }
}

Suppose thread X calls Transfer(savings, checking, 100) and gets as far as locking savings. Then we switch to thread Y which calls Transfer(checking, savings, 50). We lock checking, and then attempt to lock savings, but cannot, because X has it. We then switch back to X, which tries to lock checking, but cannot, because Y has it. We then wait forever. That's a deadlock.

Does locking the same object "nested" on the same thread cause a deadlock?

No. The answer which says that is wrong. Taking a lock you already have on the thread automatically succeeds.

Are there better techniques I should be using?

Yes. Multithreaded programs are hard to write correctly. If you must, use high-level objects such as multithreaded collections or immutable collections that are designed to solve these problems efficiently without explicit locks.

You should also read

Confusion about the lock statement in C#

Upvotes: 7

Jon Hanna
Jon Hanna

Reputation: 113242

That won't deadlock, unless there's something calling from the commented out portion into the other method and so potentially leading to the case where one thread has the first lock and is waiting for the second, while another has the second and is waiting for the first.

The big problem here is the locks are not protecting the list. The reason you need the lock in the first place is that List<T> wasn't designed for concurrent use, so you need to serialise access. Since the Add and Remove methods both involve copying elements within arrays, maintaining counts and swapping one internal for another on growing past its capacity there are plenty of opportunities for a simultaneous add and remove to fail to add, fail to remove, mess up the internal count and either have a mysteriously added null or something removed that shouldn't be. More generally there's not even a guarantee that it won't be put into a state its other code assumes is impossible and result in weird errors later on.

You need to protect the list from either method in both methods, so they need to use the same lock.

Upvotes: 3

Related Questions