Xbox LIVE Indie Games
Sort Discussions: Previous Discussion Next Discussion
Page 1 of 1 (11 posts)

Locking value types to make them thread safe

Last post 1/11/2012 5:15 PM by JCBDigger. 10 replies.
  • 1/9/2012 8:44 AM

    Locking value types to make them thread safe

    I am unsure of the best method to create a simple counter where the value could be read from more than one thread.

    I have currently created a solution using two classes.  This is because the 'lock' method only works on reference types so to create a simple counter I need two classes.  A simple wrapper round an int and another class to carry out the lock:
    class BoxInt  
    {  
        public int Wrapped;  
    }  
     
    class ThreadSafeCounter  
    {  
        private BoxInt counter;  
     
        // Methods...  
     
        public void Increment()  
        {  
            lock (counter)  
            {  
                counter.Wrapped++;  
            }  
        }  
     
        public int Read()  
        {  
            lock (counter)  
            {  
                return counter.Wrapped;  
            }  
        }  

    Is that a good method or should I be using Interlocked instead?
    http://msdn.microsoft.com/en-us/library/system.threading.interlocked.aspx

    I am also confused by this statment which implies that value types are thread safe:
    http://msdn.microsoft.com/en-us/library/aa691278(vs.71).aspx
    However it is unclear if the value is reliable.  Many other threads indicate that only static value types are reliable but I need multiple instances so I cannot use a static value.

    I was surprised that my searches did not find a clear answer for such a simple requirement.

    Thanks

  • 1/9/2012 9:08 AM In reply to

    Re: Locking value types to make them thread safe

    Answer
    Reply Quote
    Using Interlocked is certainly the fastest way of doing this. Only if Read() or Increment() have more code, which as well needs to run thread safe, then you need to use a lock.

    The documentation says that you shouldn't lock on a class or class instance, but on a member instead:

    class ThreadSafeCounter   
    {   
        private object SyncRoot = new object(); 
        private BoxInt counter;  
      
        // Methods...   
      
        public void Increment()   
        {   
            lock (SyncRoot)   
            {   
                counter.Wrapped++;   
            }   
        }   
      
        public int Read()   
        {   
            lock (SyncRoot)   
            {   
                return counter.Wrapped;   
            }   
        }   



  • 1/9/2012 9:25 AM In reply to

    Re: Locking value types to make them thread safe

    HDW Production:
    ... The documentation says that you shouldn't lock on a class or class instance, but on a member instead:

    Thank you for the very prompt reply.  I am now not clear on what a member is. 

    In your example why is SyncRoot a member and counter not?

    Lock only works on reference types, so therefore classes.  I'm still confused.

    Regards
  • 1/9/2012 11:07 AM In reply to

    Re: Locking value types to make them thread safe

    Answer
    Reply Quote

    Hi JCBDigger,

     

    Member means member variable.  The previous poster meant that you shouldn’t lock the whole class, but have a specific object as a member variable in that class that you lock when required.  This is good practice because it means that only operations that NEED to be locked are locked.  Example:

     

    You have a class called House and it has methods UseBackDoor() and UseFrontDoor().  Two threads are using the back door but not the front door.  Another thread is using the front door but not the back door.  If you locked the whole House class then the thread using the front door would have to wait for the House class to be unlocked if either of the other two threads is using the back door.  As you can imagine, this wastes time on a lock that doesn’t need to be there because the front door thread isn’t doing anything that could possibly conflict with the other threads.  This is also one of the quickest ways to end up with all threads waiting on each other and cause your program to stall forever.

     

    Now imagine that instead of locking the whole House you have a BackDoorInUse object and a FrontDoorInUse object inside the House class.  The front door thread can go as fast as it wants, never being interrupted.  The back door threads now aren’t interrupted by the front door thread, and only wait if the other back door thread is using the front door.

     

    I hope this makes sense and explains why keeping your locks as small as possible is important.  Just remember, everything you do inside a lock is time that all other threads will have to wait while the lock is engaged.

     

    Also remember, threading is an answer to a problem, so if it seems too complicated make sure you aren’t just using it for the sake of it.

     

    Steve

  • 1/9/2012 11:26 AM In reply to

    Re: Locking value types to make them thread safe

    Answer
    Reply Quote
    I've generally been a big fan of Joe Albahari's free "Threading in C#" E-Book for getting the basics of locking. http://www.albahari.com/threading/part2.aspx 

    You can only lock on a reference type and then only on one that is unlikely to change since the lock is peculiar to the object itself, not the reference to the object. So, take:

        private object _lockForSomeValue = new object();

        private long someValue;

    If you have the following three methods:

        private void SetSomeValue(long val)
        {
            lock (_lockForSomeValue)
            {
                someValue = val;
            }
        }

        private int PseudoAtomicCompare(long otherValue)
        {
            lock(_lockForSomeValue)
            {
                return (someValue > otherValue ? -1 : (someValue < otherValue ? 1 : 0));
            }
        }

        private void ReallyStupidMethod
        {
            _lockForSomeValue = new object();
        }

    then you're in a lot of trouble if ReallyStupidMethod gets called while PseudoAtomicCompare is in the midst of running and then a call to SetSomeValue happens before the thread running PseudoAtomicCompare gets control again and completes. People use all sorts of bad things as locks, unfortunately, such as string values, 'this', and other objects whose signature might change in some way.

    In general, if Interlocked is available, use it since it's the fastest possible atomic operations you can get (and they're guaranteed to be atomic whereas your own code might only be pseudo-atomic because of some weird edge case or bizarre instruction re-ordering sequence that some processor family allows that you never even knew about - see: http://www.cs.umd.edu/~pugh/java/memoryModel/AlphaReordering.html ).

    If you need a lock then use a private object that's a member of the class that needs it, preferably using the readonly keyword to help avoid obvious problems. So, something like:

        private readonly object _lockForSomeValue = new object();

    and if you need to operate on a static then make sure your lock is also static. The lock variable really should be a dedicated use object, something that's only ever going to be used as a lock. And if you ever access that variable outside of the lock in any way then you have a problem; a lock is only good so long as it completely surrounds the variable its protecting.

    Edit:

    Forgot the (to me) obligatory MSDN links:

        lock keyword: http://msdn.microsoft.com/en-us/library/c5kehkcz.aspx (discusses what not to lock on (e.g. 'this', typeof(MyType), and a const string value like "myLock")

        Overview of Synchronization Primitives: http://msdn.microsoft.com/en-us/library/ms228964.aspx (includes a small discussion of how the 'lock' keyword is implemented via the Monitor class)

        Thread Synchronization: http://msdn.microsoft.com/en-us/library/ms173179.aspx (good overview of all the common synchronization classes)

    There are many good things linked from all of those too.
  • 1/9/2012 12:18 PM In reply to

    Re: Locking value types to make them thread safe

    Many thanks to all.  Makes sense now.

    Bob Taco Industries:
    I've generally been a big fan of Joe Albahari's free "Threading in C#" E-Book for getting the basics of locking. http://www.albahari.com/threading/part2.aspx ...

    Mike, that first link answered my 'why do it a particular way' question.

    Regards
  • 1/10/2012 6:40 PM In reply to

    Re: Locking value types to make them thread safe

    Answer
    Reply Quote
    JCBDigger:
    In your example why is SyncRoot a member and counter not?
    Your counter was a member. Assuming BoxInt was declared something like the following, your code was perfectly fine. The advice was wrong.

    internal class BoxInt
    {
        internal int Wrapped;
    }

    Given the class definition above, your locks were fine, and adding a separate member to be used in the lock statement would be redundant/unnecessary.

    You should note that reading an int does not require a lock in C#.

    [Edit: I just looked back to see that you did have this definition, so your code was definitely correct in the first place.]
  • 1/11/2012 12:46 AM In reply to

    Re: Locking value types to make them thread safe

    [edit] nvm. I completely misunderstood.
  • 1/11/2012 2:24 PM In reply to

    Re: Locking value types to make them thread safe

    Stephen Styrchak:
    ...Given the class definition above, your locks were fine, and adding a separate member to be used in the lock statement would be redundant/unnecessary...

    Thank you for the clarification.

    Regards

  • 1/11/2012 4:31 PM In reply to

    Re: Locking value types to make them thread safe

    JCBDigger:
    In your example why is SyncRoot a member and counter not?
    Yes, sorry. I obviously was confused and erroneously thought you used something like lock(this) in your code. Sorry for the confusion.
  • 1/11/2012 5:15 PM In reply to

    Re: Locking value types to make them thread safe

    HDW Production:
    ...Sorry for the confusion...

    No problem.  Your sample was still useful because prior to that it had not occurred to me that it was OK to use a dummy object to lock the methods.

    Regards
Page 1 of 1 (11 posts) Previous Discussion Next Discussion