2

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.

Improve this question
6
  • Deadlock most often occurs with nested locks. However you usually want one lock to protect mutating the same object, in your case the list. Commented Sep 18, 2017 at 18:23
  • 4
    You need a deeper understanding of threading issues before you should write multi-threaded code. You're completely misunderstanding the point of locks. Commented Sep 18, 2017 at 18:25
  • current code is not going to protect list you need to use same object to synchronized access to list... Commented Sep 18, 2017 at 18:26
  • @SLaks yes I am learning as I go, unfortunately I have been given someone else's code to maintain. Commented Sep 18, 2017 at 18:27
  • 1
    "Learn as you go" is often a good idea, but multithreading is complex, difficult, and requires a thorough understanding of a great many issues to get correct. You will make terrible undetectable mistakes that leave your program fragile and hard to fix. Rather than learning as you go, consider studying books, taking a course, or finding a mentor who can help you with this. Commented Sep 18, 2017 at 22:14

2 Answers 2

Reset to default
7

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#

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Thanks Eric, yes its hard indeed. My current scenario is that I do not want removal from collection to wait around if updating code is taking long time. While updating collection is happening, I want removal to continue. This way I am ok if the entire list is empty before the update happens. Its a guarantee that there will be no duplicates
What do you want to happen if there is adding happening, that the removal happens later, or that it just doesn't happen?
@Zeus. Your current scenario is that you're waiting at a stoplight for the light to turn green, and you've apparently decided that you're waiting too long and you're just going to pay attention to a different stoplight for a different intersection, because you want to drive fast. Does this strike you as a good idea? It does not strike me as a good idea.
@Zeus: There are rules in multithreading and they exist for reasons. You can't just ignore them because you don't want the performance penalty of lock contention. That performance penalty is the price you pay for sharing resources on multiple threads, the same way that waiting at traffic lights sometimes is the price you pay for sharing the road with other people.
3

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.

Improve this answer

11 Comments

Understood however I need the updating part of the list to be protected. Code that removes from list should not wait until the list is updated. This is why 2 locks. Thoughts ?
@Zeus Do you consider it important for your code to actually work, or do you consider it okay for your removal to do one of the following: 1. throw an exception instead of removing the item 2. corrupt the list such that it becomes unusable and has nonsensical data in it 3. fail to remove the item from the list, but not throw any exceptions 4. remove the item from the list for a little while, but then have it show up again later 5. any number of other things I could think of off of the top of my head. 6. you get lucky and it just works. Keep in mind it won't constantly be one of those either.
@Zeus Of you're okay with one of those random options happening each time you run the code, then your option is fine.
If you don't want to wait then an approach that can work is to have a queue of pending operations you add to and let it go on, but then you need to serialise access to that queue the same way, so no gain here. If there was something else happening as well as the removal then maybe it'd be worth it.
Or it could be perhaps a case for the producer consumer pattern (the example code isn't, but code where you do something with the removed value could be). But in terms of the locks, step back: Why do you have locks at all? What are they there to protect against? Do they succeed? That's how to look at this.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.