none
Using lock inside class RRS feed

  • Question

  • Hello!

    I have some class and 2 method in it. I want to have thread-safety class, so I need to use lock and some lock object.

    And question: Can I use 1 lock object for 2 methods? And if I can't - is there any standard ways to have many lock objects without garbadge in class like this:

    class SomeClass
    {
    //...
    private object LockerForMethodA;
    private object LockerForMethodB;
    private object LockerForMethodC;
    private object LockerForMethodD;
    }

    Friday, January 24, 2014 4:37 PM

Answers

  • There's no "standard" way to do this.

    For a start it's worth noting that "locker for method x" doesn't make sense. You're not supposed to lock methods, you're supposed to lock access to "resources" that are shared among multiple threads. Resources can be many things but one thing is for sure, methods aren't resources. The usual resource you'll find in practice is an object, a List<int> for example.

    Let's look at an example:

    class Demo {
        List<int> A = new List<int>();
        List<int> B = new List<int>();
    
        public void Move(int index) {
            int a;
            bool contains;
    
            a = A[index];
    
            contains = B.Contains(a);
    
            if (contains) {
                B.Add(a);
                A.RemoveAt(index);
            }
        }
    }

    A simple class which contains 2 List<int> objects and a Move method which moves a value from A to B if B doesn't already contain that value.

    So it looks like we have 2 resources, A and B. We'll add 2 lock objects, lockA and lockB and modify the Move method to use those locks:

        lock (lockA)
            a = A[index];
        lock (lockB)
            contains = B.Contains(a);
        if (contains) {
            lock (lockB)
                B.Add(a);
            lock (lockA)
                A.RemoveAt(index);    
        }
    

    Every access to those lists is protected by a lock. It would seem that this method is thread safe. Is it?

    No. This code is thread safe with respect to accessing A and B but it fails to prevent a value from being moved only if it doesn't exist in B. It's possible that 2 threads execute B.Contains(a) one after another and both decide that the value doesn't exist and can be moved. You end up with the same value being added twice to B.

    How to solve? Well, if you decide that you need to do something then go ahead and do it, don't let other threads to sneak in:

        lock (lockA)
            a = A[index];
        lock (lockB) {
            contains = B.Contains(a);
            if (contains)
                B.Add(a);
        }
        if (contains) {
            lock (lockA)
                A.RemoveAt(index);
        }
    

    This version holds the lock for B for both B.Contains and B.Add, other threads can't modify B between Contains and Add.

    Is it correct now?

    No. Accessing A suffers from a similar but perhaps less obvious problem. At start Move gets the value from A using the index passed as parameter. At the end of the Move we need to remove that value from A but what happens if another thread modified A? It's possible that the index refers to a different value now. The solution is to extend the lock for A like we did for B:

    lock (lockA) {
        a = A[index];
        lock (lockB) {
            contains = B.Contains(a);
            if (contains)
                B.Add(a);
        }
        if (contains)
            A.RemoveAt(index);
    }
    

    Finally, this is correct (well, if I didn't miss anything). But this code uses 2 locks, is this really necessary?

    No. We started by creating a lock for each resource but it turns out that there are dependencies between those resources and that at some point both locks need to be hold for the program to be correct. We can remove lockB and rely only on lockA to protect both resources. Of course, if there are other methods in the class that use lockB to protect access to B then they'll have to be changed to use lockA.

    In general, you can always replace multiple locks with a single one. Basically you consider that the resource is your entire program memory and you have a single lock to protect access to the memory. The drawback of such an approach is that it is terrible slow. Threads will spend lots of time waiting for locks, that's bad for performance.

    • Marked as answer by Ruzik Friday, January 24, 2014 7:49 PM
    Friday, January 24, 2014 6:33 PM
    Moderator

All replies

  • There's no "standard" way to do this.

    For a start it's worth noting that "locker for method x" doesn't make sense. You're not supposed to lock methods, you're supposed to lock access to "resources" that are shared among multiple threads. Resources can be many things but one thing is for sure, methods aren't resources. The usual resource you'll find in practice is an object, a List<int> for example.

    Let's look at an example:

    class Demo {
        List<int> A = new List<int>();
        List<int> B = new List<int>();
    
        public void Move(int index) {
            int a;
            bool contains;
    
            a = A[index];
    
            contains = B.Contains(a);
    
            if (contains) {
                B.Add(a);
                A.RemoveAt(index);
            }
        }
    }

    A simple class which contains 2 List<int> objects and a Move method which moves a value from A to B if B doesn't already contain that value.

    So it looks like we have 2 resources, A and B. We'll add 2 lock objects, lockA and lockB and modify the Move method to use those locks:

        lock (lockA)
            a = A[index];
        lock (lockB)
            contains = B.Contains(a);
        if (contains) {
            lock (lockB)
                B.Add(a);
            lock (lockA)
                A.RemoveAt(index);    
        }
    

    Every access to those lists is protected by a lock. It would seem that this method is thread safe. Is it?

    No. This code is thread safe with respect to accessing A and B but it fails to prevent a value from being moved only if it doesn't exist in B. It's possible that 2 threads execute B.Contains(a) one after another and both decide that the value doesn't exist and can be moved. You end up with the same value being added twice to B.

    How to solve? Well, if you decide that you need to do something then go ahead and do it, don't let other threads to sneak in:

        lock (lockA)
            a = A[index];
        lock (lockB) {
            contains = B.Contains(a);
            if (contains)
                B.Add(a);
        }
        if (contains) {
            lock (lockA)
                A.RemoveAt(index);
        }
    

    This version holds the lock for B for both B.Contains and B.Add, other threads can't modify B between Contains and Add.

    Is it correct now?

    No. Accessing A suffers from a similar but perhaps less obvious problem. At start Move gets the value from A using the index passed as parameter. At the end of the Move we need to remove that value from A but what happens if another thread modified A? It's possible that the index refers to a different value now. The solution is to extend the lock for A like we did for B:

    lock (lockA) {
        a = A[index];
        lock (lockB) {
            contains = B.Contains(a);
            if (contains)
                B.Add(a);
        }
        if (contains)
            A.RemoveAt(index);
    }
    

    Finally, this is correct (well, if I didn't miss anything). But this code uses 2 locks, is this really necessary?

    No. We started by creating a lock for each resource but it turns out that there are dependencies between those resources and that at some point both locks need to be hold for the program to be correct. We can remove lockB and rely only on lockA to protect both resources. Of course, if there are other methods in the class that use lockB to protect access to B then they'll have to be changed to use lockA.

    In general, you can always replace multiple locks with a single one. Basically you consider that the resource is your entire program memory and you have a single lock to protect access to the memory. The drawback of such an approach is that it is terrible slow. Threads will spend lots of time waiting for locks, that's bad for performance.

    • Marked as answer by Ruzik Friday, January 24, 2014 7:49 PM
    Friday, January 24, 2014 6:33 PM
    Moderator
  • It's realy great and clear. Thank you for your help!
    Friday, January 24, 2014 7:49 PM