Why is lock this bad

Question

The MSDN documentation says that  public class SomeObject     public void SomeOperation           lock this                Access instance variables               is  a problem if the instance can be accessed publicly   I m wondering why  Is it because the lock will be held longer than necessary  Or is there some more insidious reason

User · Answer

Here s a much simpler illustration  taken from Question 34 here   why lock this  is bad and may result in deadlocks when consumer of your class also try to lock on the object  Below  only one of three thread can proceed  the other two are deadlocked    class SomeClass       public void SomeMethod int id                  lock this                          while true                                Console WriteLine  SomeClass SomeMethod      id                                    class Program       static void Main string   args                SomeClass o   new SomeClass             lock o                        for  int threadId   0  threadId  lt  3  threadId                                  Thread t   new Thread      gt                        o SomeMethod threadId                                               t Start                               Console WriteLine                 To work around  this guy used Thread TryMonitor  with timeout  instead of lock                Monitor TryEnter temp  millisecondsTimeout  ref lockWasTaken               if  lockWasTaken                                doAction                              else                               throw new Exception  Could not get lock                     https   blogs appbeat io post c-how-to-lock-without-deadlocks

User · Answer

There s also some good discussion about this here  Is this the proper use of a mutex

User · Answer

Here is some sample code that is simpler to follow  IMO    Will work in LinqPad  reference following namespaces  System Net and System Threading Tasks   Something to remember is that lock x  basically is syntactic sugar and what it does is to use Monitor Enter and then uses a try  catch  finally block to call Monitor Exit  See  https   docs microsoft com en-us dotnet api system threading monitor enter  remarks section      or use the C  lock statement  SyncLock statement in Visual Basic     which wraps the Enter and Exit methods in a try   finally block    void Main           demonstrates why locking on THIS is BADD   you should never lock on something that is publicly accessible      ClassTest test   new ClassTest        lock test    locking on the instance of ClassTest               Console WriteLine   CurrentThread  Thread CurrentThread ManagedThreadId             Parallel Invoke new Action                              gt                      this is there to just use up the current main thread                   Console WriteLine   CurrentThread  Thread CurrentThread ManagedThreadId                                      none of these will enter the lock section                   gt  test DoWorkUsingThisLock 1    this will dead lock as lock x  uses Monitor Enter                  gt  test DoWorkUsingMonitor 2     this will not dead lock as it uses Montory TryEnter                      public class ClassTest       public void DoWorkUsingThisLock int i                Console WriteLine   Start ClassTest DoWorkUsingThisLock  i  CurrentThread  Thread CurrentThread ManagedThreadId             lock this    this can be bad if someone has locked on this already  as it will cause it to be deadlocked                        Console WriteLine   Running  ClassTest DoWorkUsingThisLock  i  CurrentThread  Thread CurrentThread ManagedThreadId                 Thread Sleep 1000                     Console WriteLine   End ClassTest DoWorkUsingThisLock Done  i   CurrentThread  Thread CurrentThread ManagedThreadId                public void DoWorkUsingMonitor int i                Console WriteLine   Start ClassTest DoWorkUsingMonitor  i  CurrentThread  Thread CurrentThread ManagedThreadId             if  Monitor TryEnter this                         Console WriteLine   Running  ClassTest DoWorkUsingMonitor  i  CurrentThread  Thread CurrentThread ManagedThreadId                 Thread Sleep 1000               Monitor Exit this                     else                       Console WriteLine   Skipped lock section    i  CurrentThread  Thread CurrentThread ManagedThreadId                        Console WriteLine   End ClassTest DoWorkUsingMonitor Done  i  CurrentThread  Thread CurrentThread ManagedThreadId             Console WriteLine              Output  CurrentThread 15 CurrentThread 15 Start ClassTest DoWorkUsingMonitor 2 CurrentThread 13 Start ClassTest DoWorkUsingThisLock 1 CurrentThread 12 Skipped lock section   2 CurrentThread 13 End ClassTest DoWorkUsingMonitor Done 2 CurrentThread 13   Notice that Thread 12 never ends as its dead locked

User · Answer

Imagine that you have a skilled secretary at your office that s a shared resource in the department  Once in a while  you rush towards them because you have a task  only to hope that another one of your co-workers has not already claimed them  Usually you only have to wait for a brief period of time    Because caring is sharing  your manager decides that customers can use the secretary directly as well  But this has a side effect  A customer might even claim them while you re working for this customer and you also need them to execute part of the tasks  A deadlock occurs  because claiming is no longer a hierarchy  This could have been avoided all together by not allowing customers to claim them in the first place    lock this  is bad as we ve seen  An outside object might lock on the object and since you don t control who s using the class  anyone can lock on it    Which is the exact example as described above  Again  the solution is to limit exposure of the object  However  if you have a private  protected or internal class you could already control who is locking on your object  because you re sure that you ve written your code yourself  So the message here is  don t expose it as public  Also  ensuring that a lock is used in similar scenario s avoids deadlocks   The complete opposite of this is to lock on resources that are shared throughout the app domain -- the worst case scenario  It s like putting your secretary outside and allowing everyone out there to claim them  The result is utter chaos - or in terms of source code  it was a bad idea  throw it away and start over  So how do we do that   Types are shared in the app domain as most people here point out  But there are even better things we can use  strings  The reason is that strings are pooled  In other words  if you have two strings that have the same contents in an app domain  there s a chance that they have the exact same pointer  Since the pointer is used as the lock key  what you basically get is a synonym for  prepare for undefined behavior    Similarly  you shouldn t lock on WCF objects  HttpContext Current  Thread Current  Singletons  in general   etc  The easiest way to avoid all of this  private  static  object myLock   new object

User · Answer

Because any chunk of code that can see the instance of your class can also lock on that reference  You want to hide  encapsulate  your locking object so that only code that needs to reference it can reference it  The keyword this refers to the current class instance  so any number of things could have reference to it and could use it to do thread synchronization   To be clear  this is bad because some other chunk of code could use the class instance to lock  and might prevent your code from obtaining a timely lock or could create other thread sync problems  Best case  nothing else uses a reference to your class to lock  Middle case  something uses a reference to your class to do locks and it causes performance problems  Worst case  something uses a reference of your class to do locks and it causes really bad  really subtle  really hard-to-debug problems

User · Answer

and the exact same arguments apply to this construct as well   lock typeof SomeObject

User · Answer

I know this is an old thread  but because people can still look this up and rely on it  it seems important to point out that lock typeof SomeObject   is significantly worse than lock this    Having said that  sincere kudos to Alan for pointing out that lock typeof SomeObject   is bad practice   An instance of System Type is one of the most generic  coarse-grained objects there is   At the very least  an instance of System Type is global to an AppDomain  and  NET can run multiple programs in an AppDomain   This means that two entirely different programs could potentially cause interference in one another even to the extent of creating a deadlock if they both try to get a synchronization lock on the same type instance   So lock this  isn t particularly robust form  can cause problems and should always raise eyebrows for all the reasons cited   Yet there is widely used  relatively well-respected and apparently stable code like log4net that uses the lock this  pattern extensively  even though I would personally prefer to see that pattern change   But lock typeof SomeObject   opens up a whole new and enhanced can of worms   For what it s worth

User · Answer

Here is some sample code that is simpler to follow  IMO    Will work in LinqPad  reference following namespaces  System Net and System Threading Tasks   Something to remember is that lock x  basically is syntactic sugar and what it does is to use Monitor Enter and then uses a try  catch  finally block to call Monitor Exit  See  https   docs microsoft com en-us dotnet api system threading monitor enter  remarks section      or use the C  lock statement  SyncLock statement in Visual Basic     which wraps the Enter and Exit methods in a try   finally block    void Main           demonstrates why locking on THIS is BADD   you should never lock on something that is publicly accessible      ClassTest test   new ClassTest        lock test    locking on the instance of ClassTest               Console WriteLine   CurrentThread  Thread CurrentThread ManagedThreadId             Parallel Invoke new Action                              gt                      this is there to just use up the current main thread                   Console WriteLine   CurrentThread  Thread CurrentThread ManagedThreadId                                      none of these will enter the lock section                   gt  test DoWorkUsingThisLock 1    this will dead lock as lock x  uses Monitor Enter                  gt  test DoWorkUsingMonitor 2     this will not dead lock as it uses Montory TryEnter                      public class ClassTest       public void DoWorkUsingThisLock int i                Console WriteLine   Start ClassTest DoWorkUsingThisLock  i  CurrentThread  Thread CurrentThread ManagedThreadId             lock this    this can be bad if someone has locked on this already  as it will cause it to be deadlocked                        Console WriteLine   Running  ClassTest DoWorkUsingThisLock  i  CurrentThread  Thread CurrentThread ManagedThreadId                 Thread Sleep 1000                     Console WriteLine   End ClassTest DoWorkUsingThisLock Done  i   CurrentThread  Thread CurrentThread ManagedThreadId                public void DoWorkUsingMonitor int i                Console WriteLine   Start ClassTest DoWorkUsingMonitor  i  CurrentThread  Thread CurrentThread ManagedThreadId             if  Monitor TryEnter this                         Console WriteLine   Running  ClassTest DoWorkUsingMonitor  i  CurrentThread  Thread CurrentThread ManagedThreadId                 Thread Sleep 1000               Monitor Exit this                     else                       Console WriteLine   Skipped lock section    i  CurrentThread  Thread CurrentThread ManagedThreadId                        Console WriteLine   End ClassTest DoWorkUsingMonitor Done  i  CurrentThread  Thread CurrentThread ManagedThreadId             Console WriteLine              Output  CurrentThread 15 CurrentThread 15 Start ClassTest DoWorkUsingMonitor 2 CurrentThread 13 Start ClassTest DoWorkUsingThisLock 1 CurrentThread 12 Skipped lock section   2 CurrentThread 13 End ClassTest DoWorkUsingMonitor Done 2 CurrentThread 13   Notice that Thread 12 never ends as its dead locked

User · Answer

You can establish a rule that says that a class can have code that locks on  this  or any object that the code in the class instantiates    So it s only a problem if the pattern is not followed   If you want to protect yourself from code that won t follow this pattern  then the accepted answer is correct    But if the pattern is followed  it s not a problem   The advantage of lock this  is efficiency   What if you have a simple  value object  that holds a single value   It s just a wrapper  and it gets instantiated millions of times   By requiring the creation of a private sync object just for locking  you ve basically doubled the size of the object and doubled the number of allocations   When performance matters  this is an advantage   When you don t care about number of allocations or memory footprint  avoiding lock this  is preferable for the reasons indicated in other answers

User · Answer

It is bad form to use this in lock statements because it is generally out of your control who else might be locking on that object   In order to properly plan parallel operations  special care should be taken to consider possible deadlock situations  and having an unknown number of lock entry points hinders this  For example  any one with a reference to the object can lock on it without the object designer creator knowing about it  This increases the complexity of multi-threaded solutions and might affect their correctness   A private field is usually a better option as the compiler will enforce access restrictions to it  and it will encapsulate the locking mechanism  Using this violates encapsulation by exposing part of your locking implementation to the public  It is also not clear that you will be acquiring a lock on this unless it has been documented  Even then  relying on documentation to prevent a problem is sub-optimal   Finally  there is the common misconception that lock this  actually modifies the object passed as a parameter  and in some way makes it read-only or inaccessible  This is false  The object passed as a parameter to lock merely serves as a key  If a lock is already being held on that key  the lock cannot be made  otherwise  the lock is allowed   This is why it s bad to use strings as the keys in lock statements  since they are immutable and are shared accessible across parts of the application  You should use a private variable instead  an Object instance will do nicely   Run the following C  code as an example   public class Person       public int Age   get  set         public string Name   get  set         public void LockThis                 lock  this                        System Threading Thread Sleep 10000                      class Program       static void Main string   args                var nancy   new Person  Name    Nancy Drew   Age   15           var a   new Thread nancy LockThis           a Start            var b   new Thread Timewarp           b Start nancy           Thread Sleep 10           var anotherNancy   new Person   Name    Nancy Drew   Age   50            var c   new Thread NameChange           c Start anotherNancy           a Join            Console ReadLine               static void Timewarp object subject                var person   subject as Person          if  person    null  throw new ArgumentNullException  subject               A lock does not make the object read-only          lock  person Name                        while  person Age  lt   23                                   There will be a lock on  person  due to the LockThis method running in another thread                 if  Monitor TryEnter person  10     false                                        Console WriteLine   this  person is locked                                       else Monitor Exit person                   person Age                    if person Age    18                                           Changing the  person Name  value doesn t change the lock                        person Name    Nancy Smith                                     Console WriteLine   0  is  1  years old    person Name  person Age                                      static void NameChange object subject                var person   subject as Person          if  person    null  throw new ArgumentNullException  subject               You should avoid locking on strings  since they are immutable          if  Monitor TryEnter person Name  30     false                        Console WriteLine  Failed to obtain lock on 50 year old Nancy  because Timewarp object  locked on string   Nancy Drew                         else Monitor Exit person Name            if  Monitor TryEnter  Nancy Drew   30     false                        Console WriteLine  Failed to obtain lock using  Nancy Drew  literal  locked by  person Name  since both are the same object thanks to inlining                       else Monitor Exit  Nancy Drew            if  Monitor TryEnter person Name  10000                         string oldName   person Name              person Name    Nancy Callahan               Console WriteLine  Name changed from   0   to   1      oldName  person Name                     else Monitor Exit person Name             Console output   this  person is locked  Nancy Drew is 16 years old   this  person is locked  Nancy Drew is 17 years old  Failed to obtain lock on 50 year old Nancy  because Timewarp object  locked on string  Nancy Drew    this  person is locked  Nancy Smith is 18 years old   this  person is locked  Nancy Smith is 19 years old   this  person is locked  Nancy Smith is 20 years old  Failed to obtain lock using  Nancy Drew  literal  locked by  person Name  since both are the same object thanks to inlining   this  person is locked  Nancy Smith is 21 years old   this  person is locked  Nancy Smith is 22 years old   this  person is locked  Nancy Smith is 23 years old   this  person is locked  Nancy Smith is 24 years old  Name changed from  Nancy Drew  to  Nancy Callahan

User · Answer

Take a look at the MSDN Topic Thread Synchronization  C  Programming Guide      Generally  it is best to avoid locking   on a public type  or on object   instances beyond the control of your   application  For example  lock this    can be problematic if the instance can   be accessed publicly  because code   beyond your control may lock on the   object as well  This could create   deadlock situations where two or more   threads wait for the release of the   same object  Locking on a public   data type  as opposed to an object    can cause problems for the same   reason  Locking on literal strings is   especially risky because literal   strings are interned by the common   language runtime  CLR   This means   that there is one instance of any   given string literal for the entire   program  the exact same object   represents the literal in all running   application domains  on all threads    As a result  a lock placed on a string   with the same contents anywhere in the   application process locks all   instances of that string in the   application  As a result  it is best   to lock a private or protected member   that is not interned  Some classes   provide members specifically for   locking  The Array type  for example    provides SyncRoot  Many collection   types provide a SyncRoot member as   well

User · Answer

There will be a problem if the instance can be accessed publicly because there could be other requests that might be using the same object instance  It s better to use private static variable

User · Answer

Imagine that you have a skilled secretary at your office that s a shared resource in the department  Once in a while  you rush towards them because you have a task  only to hope that another one of your co-workers has not already claimed them  Usually you only have to wait for a brief period of time    Because caring is sharing  your manager decides that customers can use the secretary directly as well  But this has a side effect  A customer might even claim them while you re working for this customer and you also need them to execute part of the tasks  A deadlock occurs  because claiming is no longer a hierarchy  This could have been avoided all together by not allowing customers to claim them in the first place    lock this  is bad as we ve seen  An outside object might lock on the object and since you don t control who s using the class  anyone can lock on it    Which is the exact example as described above  Again  the solution is to limit exposure of the object  However  if you have a private  protected or internal class you could already control who is locking on your object  because you re sure that you ve written your code yourself  So the message here is  don t expose it as public  Also  ensuring that a lock is used in similar scenario s avoids deadlocks   The complete opposite of this is to lock on resources that are shared throughout the app domain -- the worst case scenario  It s like putting your secretary outside and allowing everyone out there to claim them  The result is utter chaos - or in terms of source code  it was a bad idea  throw it away and start over  So how do we do that   Types are shared in the app domain as most people here point out  But there are even better things we can use  strings  The reason is that strings are pooled  In other words  if you have two strings that have the same contents in an app domain  there s a chance that they have the exact same pointer  Since the pointer is used as the lock key  what you basically get is a synonym for  prepare for undefined behavior    Similarly  you shouldn t lock on WCF objects  HttpContext Current  Thread Current  Singletons  in general   etc  The easiest way to avoid all of this  private  static  object myLock   new object

User · Answer

Because if people can get at your object instance  ie  your this  pointer  then they can also try to lock that same object  Now they might not be aware that you re locking on this internally  so this may cause problems  possibly a deadlock   In addition to this  it s also bad practice  because it s locking  too much   For example  you might have a member variable of List lt int gt   and the only thing you actually need to lock is that member variable  If you lock the entire object in your functions  then other things which call those functions will be blocked waiting for the lock  If those functions don t need to access the member list  you ll be causing other code to wait and slow down your application for no reason at all

User · Answer

Because any chunk of code that can see the instance of your class can also lock on that reference  You want to hide  encapsulate  your locking object so that only code that needs to reference it can reference it  The keyword this refers to the current class instance  so any number of things could have reference to it and could use it to do thread synchronization   To be clear  this is bad because some other chunk of code could use the class instance to lock  and might prevent your code from obtaining a timely lock or could create other thread sync problems  Best case  nothing else uses a reference to your class to lock  Middle case  something uses a reference to your class to do locks and it causes performance problems  Worst case  something uses a reference of your class to do locks and it causes really bad  really subtle  really hard-to-debug problems

User · Answer

Sorry guys but I can t agree with the argument that locking this might cause deadlock  You are confusing two things  deadlocking and starving    You cannot cancel deadlock without interrupting one of the threads so after you get into a deadlock you cannot get out Starving will end automatically after one of the threads finishes its job   Here is a picture which illustrates the difference   Conclusion You can still safely use lock this  if thread starvation is not an issue for you  You still have to keep in mind that when the thread  which is starving thread using lock this  ends in a lock having your object locked  it will finally end in eternal starvation

User · Answer

Because if people can get at your object instance  ie  your this  pointer  then they can also try to lock that same object  Now they might not be aware that you re locking on this internally  so this may cause problems  possibly a deadlock   In addition to this  it s also bad practice  because it s locking  too much   For example  you might have a member variable of List lt int gt   and the only thing you actually need to lock is that member variable  If you lock the entire object in your functions  then other things which call those functions will be blocked waiting for the lock  If those functions don t need to access the member list  you ll be causing other code to wait and slow down your application for no reason at all

User · Answer

and the exact same arguments apply to this construct as well   lock typeof SomeObject

User · Answer

Take a look at the MSDN Topic Thread Synchronization  C  Programming Guide      Generally  it is best to avoid locking   on a public type  or on object   instances beyond the control of your   application  For example  lock this    can be problematic if the instance can   be accessed publicly  because code   beyond your control may lock on the   object as well  This could create   deadlock situations where two or more   threads wait for the release of the   same object  Locking on a public   data type  as opposed to an object    can cause problems for the same   reason  Locking on literal strings is   especially risky because literal   strings are interned by the common   language runtime  CLR   This means   that there is one instance of any   given string literal for the entire   program  the exact same object   represents the literal in all running   application domains  on all threads    As a result  a lock placed on a string   with the same contents anywhere in the   application process locks all   instances of that string in the   application  As a result  it is best   to lock a private or protected member   that is not interned  Some classes   provide members specifically for   locking  The Array type  for example    provides SyncRoot  Many collection   types provide a SyncRoot member as   well

User · Answer

There will be a problem if the instance can be accessed publicly because there could be other requests that might be using the same object instance  It s better to use private static variable

User · Answer

Because if people can get at your object instance  ie  your this  pointer  then they can also try to lock that same object  Now they might not be aware that you re locking on this internally  so this may cause problems  possibly a deadlock   In addition to this  it s also bad practice  because it s locking  too much   For example  you might have a member variable of List lt int gt   and the only thing you actually need to lock is that member variable  If you lock the entire object in your functions  then other things which call those functions will be blocked waiting for the lock  If those functions don t need to access the member list  you ll be causing other code to wait and slow down your application for no reason at all

User · Answer

Locking on the this pointer can be bad if you are locking over a shared resource   A shared resource can be a static variable or a file on your computer - i e  something that is shared between all users of the class   The reason is that the this pointer will contain a different reference to a location in memory each time your class is instantiated   So  locking over this in once instance of a class is different than locking over this in another instance of a class   Check out this code to see what I mean   Add the following code to your main program in a Console application       static void Main string   args                 TestThreading             Console ReadLine               public static void TestThreading                 Random rand   new Random            Thread   threads   new Thread 10           TestLock balance   100000          for  int i   0  i  lt  10  i                          TestLock tl   new TestLock                Thread t   new Thread new ThreadStart tl WithdrawAmount                threads i    t                    for  int i   0  i  lt  10  i                          threads i  Start                      Console Read            Create a new class like the below    class TestLock       public static int balance   get  set        public static readonly Object myLock   new Object         public void Withdraw int amount                 Try both locks to see what I mean                      lock  this         lock  myLock                        Random rand   new Random                if  balance  gt   amount                                Console WriteLine  Balance before Withdrawal        balance                   Console WriteLine  Withdraw          -    amount                   balance   balance - amount                  Console WriteLine  Balance after Withdrawal         balance                             else                               Console WriteLine  Can t process your transaction  current balance is        balance     and you tried to withdraw     amount                                      public void WithdrawAmount                 Random rand   new Random            Withdraw rand Next 1  100    100             Here is a run of the program locking on this      Balance before Withdrawal    100000     Withdraw          -5600     Balance after Withdrawal     94400     Balance before Withdrawal    100000     Balance before Withdrawal    100000     Withdraw          -5600     Balance after Withdrawal     88800     Withdraw          -5600     Balance after Withdrawal     83200     Balance before Withdrawal    83200     Withdraw          -9100     Balance after Withdrawal     74100     Balance before Withdrawal    74100     Withdraw          -9100     Balance before Withdrawal    74100     Withdraw          -9100     Balance after Withdrawal     55900     Balance after Withdrawal     65000     Balance before Withdrawal    55900     Withdraw          -9100     Balance after Withdrawal     46800     Balance before Withdrawal    46800     Withdraw          -2800     Balance after Withdrawal     44000     Balance before Withdrawal    44000     Withdraw          -2800     Balance after Withdrawal     41200     Balance before Withdrawal    44000     Withdraw          -2800     Balance after Withdrawal     38400   Here is a run of the program locking on myLock   Balance before Withdrawal    100000 Withdraw          -6600 Balance after Withdrawal     93400 Balance before Withdrawal    93400 Withdraw          -6600 Balance after Withdrawal     86800 Balance before Withdrawal    86800 Withdraw          -200 Balance after Withdrawal     86600 Balance before Withdrawal    86600 Withdraw          -8500 Balance after Withdrawal     78100 Balance before Withdrawal    78100 Withdraw          -8500 Balance after Withdrawal     69600 Balance before Withdrawal    69600 Withdraw          -8500 Balance after Withdrawal     61100 Balance before Withdrawal    61100 Withdraw          -2200 Balance after Withdrawal     58900 Balance before Withdrawal    58900 Withdraw          -2200 Balance after Withdrawal     56700 Balance before Withdrawal    56700 Withdraw          -2200 Balance after Withdrawal     54500 Balance before Withdrawal    54500 Withdraw          -500 Balance after Withdrawal     54000

User · Answer

Sorry guys but I can t agree with the argument that locking this might cause deadlock  You are confusing two things  deadlocking and starving    You cannot cancel deadlock without interrupting one of the threads so after you get into a deadlock you cannot get out Starving will end automatically after one of the threads finishes its job   Here is a picture which illustrates the difference   Conclusion You can still safely use lock this  if thread starvation is not an issue for you  You still have to keep in mind that when the thread  which is starving thread using lock this  ends in a lock having your object locked  it will finally end in eternal starvation

User · Answer

and the exact same arguments apply to this construct as well   lock typeof SomeObject

User · Answer

Please refer to the following link which explains why lock  this  is not a good idea   https   docs microsoft com en-us dotnet standard threading managed-threading-best-practices  So the solution is to add a private object  for example  lockObject to the class and place the code region inside the lock statement as shown below   lock  lockObject

User · Answer

Because any chunk of code that can see the instance of your class can also lock on that reference  You want to hide  encapsulate  your locking object so that only code that needs to reference it can reference it  The keyword this refers to the current class instance  so any number of things could have reference to it and could use it to do thread synchronization   To be clear  this is bad because some other chunk of code could use the class instance to lock  and might prevent your code from obtaining a timely lock or could create other thread sync problems  Best case  nothing else uses a reference to your class to lock  Middle case  something uses a reference to your class to do locks and it causes performance problems  Worst case  something uses a reference of your class to do locks and it causes really bad  really subtle  really hard-to-debug problems

User · Answer

Locking on the this pointer can be bad if you are locking over a shared resource   A shared resource can be a static variable or a file on your computer - i e  something that is shared between all users of the class   The reason is that the this pointer will contain a different reference to a location in memory each time your class is instantiated   So  locking over this in once instance of a class is different than locking over this in another instance of a class   Check out this code to see what I mean   Add the following code to your main program in a Console application       static void Main string   args                 TestThreading             Console ReadLine               public static void TestThreading                 Random rand   new Random            Thread   threads   new Thread 10           TestLock balance   100000          for  int i   0  i  lt  10  i                          TestLock tl   new TestLock                Thread t   new Thread new ThreadStart tl WithdrawAmount                threads i    t                    for  int i   0  i  lt  10  i                          threads i  Start                      Console Read            Create a new class like the below    class TestLock       public static int balance   get  set        public static readonly Object myLock   new Object         public void Withdraw int amount                 Try both locks to see what I mean                      lock  this         lock  myLock                        Random rand   new Random                if  balance  gt   amount                                Console WriteLine  Balance before Withdrawal        balance                   Console WriteLine  Withdraw          -    amount                   balance   balance - amount                  Console WriteLine  Balance after Withdrawal         balance                             else                               Console WriteLine  Can t process your transaction  current balance is        balance     and you tried to withdraw     amount                                      public void WithdrawAmount                 Random rand   new Random            Withdraw rand Next 1  100    100             Here is a run of the program locking on this      Balance before Withdrawal    100000     Withdraw          -5600     Balance after Withdrawal     94400     Balance before Withdrawal    100000     Balance before Withdrawal    100000     Withdraw          -5600     Balance after Withdrawal     88800     Withdraw          -5600     Balance after Withdrawal     83200     Balance before Withdrawal    83200     Withdraw          -9100     Balance after Withdrawal     74100     Balance before Withdrawal    74100     Withdraw          -9100     Balance before Withdrawal    74100     Withdraw          -9100     Balance after Withdrawal     55900     Balance after Withdrawal     65000     Balance before Withdrawal    55900     Withdraw          -9100     Balance after Withdrawal     46800     Balance before Withdrawal    46800     Withdraw          -2800     Balance after Withdrawal     44000     Balance before Withdrawal    44000     Withdraw          -2800     Balance after Withdrawal     41200     Balance before Withdrawal    44000     Withdraw          -2800     Balance after Withdrawal     38400   Here is a run of the program locking on myLock   Balance before Withdrawal    100000 Withdraw          -6600 Balance after Withdrawal     93400 Balance before Withdrawal    93400 Withdraw          -6600 Balance after Withdrawal     86800 Balance before Withdrawal    86800 Withdraw          -200 Balance after Withdrawal     86600 Balance before Withdrawal    86600 Withdraw          -8500 Balance after Withdrawal     78100 Balance before Withdrawal    78100 Withdraw          -8500 Balance after Withdrawal     69600 Balance before Withdrawal    69600 Withdraw          -8500 Balance after Withdrawal     61100 Balance before Withdrawal    61100 Withdraw          -2200 Balance after Withdrawal     58900 Balance before Withdrawal    58900 Withdraw          -2200 Balance after Withdrawal     56700 Balance before Withdrawal    56700 Withdraw          -2200 Balance after Withdrawal     54500 Balance before Withdrawal    54500 Withdraw          -500 Balance after Withdrawal     54000

User · Answer

There s also some good discussion about this here  Is this the proper use of a mutex

User · Answer

There s also some good discussion about this here  Is this the proper use of a mutex

User · Answer

Take a look at the MSDN Topic Thread Synchronization  C  Programming Guide      Generally  it is best to avoid locking   on a public type  or on object   instances beyond the control of your   application  For example  lock this    can be problematic if the instance can   be accessed publicly  because code   beyond your control may lock on the   object as well  This could create   deadlock situations where two or more   threads wait for the release of the   same object  Locking on a public   data type  as opposed to an object    can cause problems for the same   reason  Locking on literal strings is   especially risky because literal   strings are interned by the common   language runtime  CLR   This means   that there is one instance of any   given string literal for the entire   program  the exact same object   represents the literal in all running   application domains  on all threads    As a result  a lock placed on a string   with the same contents anywhere in the   application process locks all   instances of that string in the   application  As a result  it is best   to lock a private or protected member   that is not interned  Some classes   provide members specifically for   locking  The Array type  for example    provides SyncRoot  Many collection   types provide a SyncRoot member as   well

User · Answer

There s also some good discussion about this here  Is this the proper use of a mutex

User · Answer

and the exact same arguments apply to this construct as well   lock typeof SomeObject

User · Answer

You can establish a rule that says that a class can have code that locks on  this  or any object that the code in the class instantiates    So it s only a problem if the pattern is not followed   If you want to protect yourself from code that won t follow this pattern  then the accepted answer is correct    But if the pattern is followed  it s not a problem   The advantage of lock this  is efficiency   What if you have a simple  value object  that holds a single value   It s just a wrapper  and it gets instantiated millions of times   By requiring the creation of a private sync object just for locking  you ve basically doubled the size of the object and doubled the number of allocations   When performance matters  this is an advantage   When you don t care about number of allocations or memory footprint  avoiding lock this  is preferable for the reasons indicated in other answers

User · Answer

It is bad form to use this in lock statements because it is generally out of your control who else might be locking on that object   In order to properly plan parallel operations  special care should be taken to consider possible deadlock situations  and having an unknown number of lock entry points hinders this  For example  any one with a reference to the object can lock on it without the object designer creator knowing about it  This increases the complexity of multi-threaded solutions and might affect their correctness   A private field is usually a better option as the compiler will enforce access restrictions to it  and it will encapsulate the locking mechanism  Using this violates encapsulation by exposing part of your locking implementation to the public  It is also not clear that you will be acquiring a lock on this unless it has been documented  Even then  relying on documentation to prevent a problem is sub-optimal   Finally  there is the common misconception that lock this  actually modifies the object passed as a parameter  and in some way makes it read-only or inaccessible  This is false  The object passed as a parameter to lock merely serves as a key  If a lock is already being held on that key  the lock cannot be made  otherwise  the lock is allowed   This is why it s bad to use strings as the keys in lock statements  since they are immutable and are shared accessible across parts of the application  You should use a private variable instead  an Object instance will do nicely   Run the following C  code as an example   public class Person       public int Age   get  set         public string Name   get  set         public void LockThis                 lock  this                        System Threading Thread Sleep 10000                      class Program       static void Main string   args                var nancy   new Person  Name    Nancy Drew   Age   15           var a   new Thread nancy LockThis           a Start            var b   new Thread Timewarp           b Start nancy           Thread Sleep 10           var anotherNancy   new Person   Name    Nancy Drew   Age   50            var c   new Thread NameChange           c Start anotherNancy           a Join            Console ReadLine               static void Timewarp object subject                var person   subject as Person          if  person    null  throw new ArgumentNullException  subject               A lock does not make the object read-only          lock  person Name                        while  person Age  lt   23                                   There will be a lock on  person  due to the LockThis method running in another thread                 if  Monitor TryEnter person  10     false                                        Console WriteLine   this  person is locked                                       else Monitor Exit person                   person Age                    if person Age    18                                           Changing the  person Name  value doesn t change the lock                        person Name    Nancy Smith                                     Console WriteLine   0  is  1  years old    person Name  person Age                                      static void NameChange object subject                var person   subject as Person          if  person    null  throw new ArgumentNullException  subject               You should avoid locking on strings  since they are immutable          if  Monitor TryEnter person Name  30     false                        Console WriteLine  Failed to obtain lock on 50 year old Nancy  because Timewarp object  locked on string   Nancy Drew                         else Monitor Exit person Name            if  Monitor TryEnter  Nancy Drew   30     false                        Console WriteLine  Failed to obtain lock using  Nancy Drew  literal  locked by  person Name  since both are the same object thanks to inlining                       else Monitor Exit  Nancy Drew            if  Monitor TryEnter person Name  10000                         string oldName   person Name              person Name    Nancy Callahan               Console WriteLine  Name changed from   0   to   1      oldName  person Name                     else Monitor Exit person Name             Console output   this  person is locked  Nancy Drew is 16 years old   this  person is locked  Nancy Drew is 17 years old  Failed to obtain lock on 50 year old Nancy  because Timewarp object  locked on string  Nancy Drew    this  person is locked  Nancy Smith is 18 years old   this  person is locked  Nancy Smith is 19 years old   this  person is locked  Nancy Smith is 20 years old  Failed to obtain lock using  Nancy Drew  literal  locked by  person Name  since both are the same object thanks to inlining   this  person is locked  Nancy Smith is 21 years old   this  person is locked  Nancy Smith is 22 years old   this  person is locked  Nancy Smith is 23 years old   this  person is locked  Nancy Smith is 24 years old  Name changed from  Nancy Drew  to  Nancy Callahan

User · Answer

I know this is an old thread  but because people can still look this up and rely on it  it seems important to point out that lock typeof SomeObject   is significantly worse than lock this    Having said that  sincere kudos to Alan for pointing out that lock typeof SomeObject   is bad practice   An instance of System Type is one of the most generic  coarse-grained objects there is   At the very least  an instance of System Type is global to an AppDomain  and  NET can run multiple programs in an AppDomain   This means that two entirely different programs could potentially cause interference in one another even to the extent of creating a deadlock if they both try to get a synchronization lock on the same type instance   So lock this  isn t particularly robust form  can cause problems and should always raise eyebrows for all the reasons cited   Yet there is widely used  relatively well-respected and apparently stable code like log4net that uses the lock this  pattern extensively  even though I would personally prefer to see that pattern change   But lock typeof SomeObject   opens up a whole new and enhanced can of worms   For what it s worth

User · Answer

It is bad form to use this in lock statements because it is generally out of your control who else might be locking on that object   In order to properly plan parallel operations  special care should be taken to consider possible deadlock situations  and having an unknown number of lock entry points hinders this  For example  any one with a reference to the object can lock on it without the object designer creator knowing about it  This increases the complexity of multi-threaded solutions and might affect their correctness   A private field is usually a better option as the compiler will enforce access restrictions to it  and it will encapsulate the locking mechanism  Using this violates encapsulation by exposing part of your locking implementation to the public  It is also not clear that you will be acquiring a lock on this unless it has been documented  Even then  relying on documentation to prevent a problem is sub-optimal   Finally  there is the common misconception that lock this  actually modifies the object passed as a parameter  and in some way makes it read-only or inaccessible  This is false  The object passed as a parameter to lock merely serves as a key  If a lock is already being held on that key  the lock cannot be made  otherwise  the lock is allowed   This is why it s bad to use strings as the keys in lock statements  since they are immutable and are shared accessible across parts of the application  You should use a private variable instead  an Object instance will do nicely   Run the following C  code as an example   public class Person       public int Age   get  set         public string Name   get  set         public void LockThis                 lock  this                        System Threading Thread Sleep 10000                      class Program       static void Main string   args                var nancy   new Person  Name    Nancy Drew   Age   15           var a   new Thread nancy LockThis           a Start            var b   new Thread Timewarp           b Start nancy           Thread Sleep 10           var anotherNancy   new Person   Name    Nancy Drew   Age   50            var c   new Thread NameChange           c Start anotherNancy           a Join            Console ReadLine               static void Timewarp object subject                var person   subject as Person          if  person    null  throw new ArgumentNullException  subject               A lock does not make the object read-only          lock  person Name                        while  person Age  lt   23                                   There will be a lock on  person  due to the LockThis method running in another thread                 if  Monitor TryEnter person  10     false                                        Console WriteLine   this  person is locked                                       else Monitor Exit person                   person Age                    if person Age    18                                           Changing the  person Name  value doesn t change the lock                        person Name    Nancy Smith                                     Console WriteLine   0  is  1  years old    person Name  person Age                                      static void NameChange object subject                var person   subject as Person          if  person    null  throw new ArgumentNullException  subject               You should avoid locking on strings  since they are immutable          if  Monitor TryEnter person Name  30     false                        Console WriteLine  Failed to obtain lock on 50 year old Nancy  because Timewarp object  locked on string   Nancy Drew                         else Monitor Exit person Name            if  Monitor TryEnter  Nancy Drew   30     false                        Console WriteLine  Failed to obtain lock using  Nancy Drew  literal  locked by  person Name  since both are the same object thanks to inlining                       else Monitor Exit  Nancy Drew            if  Monitor TryEnter person Name  10000                         string oldName   person Name              person Name    Nancy Callahan               Console WriteLine  Name changed from   0   to   1      oldName  person Name                     else Monitor Exit person Name             Console output   this  person is locked  Nancy Drew is 16 years old   this  person is locked  Nancy Drew is 17 years old  Failed to obtain lock on 50 year old Nancy  because Timewarp object  locked on string  Nancy Drew    this  person is locked  Nancy Smith is 18 years old   this  person is locked  Nancy Smith is 19 years old   this  person is locked  Nancy Smith is 20 years old  Failed to obtain lock using  Nancy Drew  literal  locked by  person Name  since both are the same object thanks to inlining   this  person is locked  Nancy Smith is 21 years old   this  person is locked  Nancy Smith is 22 years old   this  person is locked  Nancy Smith is 23 years old   this  person is locked  Nancy Smith is 24 years old  Name changed from  Nancy Drew  to  Nancy Callahan

User · Answer

There is very good article about it http   bytes com topic c-sharp answers 249277-dont-lock-type-objects by Rico Mariani  performance architect for the Microsoft    NET runtime  Excerpt      The basic problem here is that you don t own the type object  and you   don t know who else could access it  In general  it s a very bad idea   to rely on locking an object you didn t create and don t know who else   might be accessing  Doing so invites deadlock  The safest way is to   only lock private objects

User · Answer

It is bad form to use this in lock statements because it is generally out of your control who else might be locking on that object   In order to properly plan parallel operations  special care should be taken to consider possible deadlock situations  and having an unknown number of lock entry points hinders this  For example  any one with a reference to the object can lock on it without the object designer creator knowing about it  This increases the complexity of multi-threaded solutions and might affect their correctness   A private field is usually a better option as the compiler will enforce access restrictions to it  and it will encapsulate the locking mechanism  Using this violates encapsulation by exposing part of your locking implementation to the public  It is also not clear that you will be acquiring a lock on this unless it has been documented  Even then  relying on documentation to prevent a problem is sub-optimal   Finally  there is the common misconception that lock this  actually modifies the object passed as a parameter  and in some way makes it read-only or inaccessible  This is false  The object passed as a parameter to lock merely serves as a key  If a lock is already being held on that key  the lock cannot be made  otherwise  the lock is allowed   This is why it s bad to use strings as the keys in lock statements  since they are immutable and are shared accessible across parts of the application  You should use a private variable instead  an Object instance will do nicely   Run the following C  code as an example   public class Person       public int Age   get  set         public string Name   get  set         public void LockThis                 lock  this                        System Threading Thread Sleep 10000                      class Program       static void Main string   args                var nancy   new Person  Name    Nancy Drew   Age   15           var a   new Thread nancy LockThis           a Start            var b   new Thread Timewarp           b Start nancy           Thread Sleep 10           var anotherNancy   new Person   Name    Nancy Drew   Age   50            var c   new Thread NameChange           c Start anotherNancy           a Join            Console ReadLine               static void Timewarp object subject                var person   subject as Person          if  person    null  throw new ArgumentNullException  subject               A lock does not make the object read-only          lock  person Name                        while  person Age  lt   23                                   There will be a lock on  person  due to the LockThis method running in another thread                 if  Monitor TryEnter person  10     false                                        Console WriteLine   this  person is locked                                       else Monitor Exit person                   person Age                    if person Age    18                                           Changing the  person Name  value doesn t change the lock                        person Name    Nancy Smith                                     Console WriteLine   0  is  1  years old    person Name  person Age                                      static void NameChange object subject                var person   subject as Person          if  person    null  throw new ArgumentNullException  subject               You should avoid locking on strings  since they are immutable          if  Monitor TryEnter person Name  30     false                        Console WriteLine  Failed to obtain lock on 50 year old Nancy  because Timewarp object  locked on string   Nancy Drew                         else Monitor Exit person Name            if  Monitor TryEnter  Nancy Drew   30     false                        Console WriteLine  Failed to obtain lock using  Nancy Drew  literal  locked by  person Name  since both are the same object thanks to inlining                       else Monitor Exit  Nancy Drew            if  Monitor TryEnter person Name  10000                         string oldName   person Name              person Name    Nancy Callahan               Console WriteLine  Name changed from   0   to   1      oldName  person Name                     else Monitor Exit person Name             Console output   this  person is locked  Nancy Drew is 16 years old   this  person is locked  Nancy Drew is 17 years old  Failed to obtain lock on 50 year old Nancy  because Timewarp object  locked on string  Nancy Drew    this  person is locked  Nancy Smith is 18 years old   this  person is locked  Nancy Smith is 19 years old   this  person is locked  Nancy Smith is 20 years old  Failed to obtain lock using  Nancy Drew  literal  locked by  person Name  since both are the same object thanks to inlining   this  person is locked  Nancy Smith is 21 years old   this  person is locked  Nancy Smith is 22 years old   this  person is locked  Nancy Smith is 23 years old   this  person is locked  Nancy Smith is 24 years old  Name changed from  Nancy Drew  to  Nancy Callahan

User · Answer

There is very good article about it http   bytes com topic c-sharp answers 249277-dont-lock-type-objects by Rico Mariani  performance architect for the Microsoft    NET runtime  Excerpt      The basic problem here is that you don t own the type object  and you   don t know who else could access it  In general  it s a very bad idea   to rely on locking an object you didn t create and don t know who else   might be accessing  Doing so invites deadlock  The safest way is to   only lock private objects

User · Answer

Because if people can get at your object instance  ie  your this  pointer  then they can also try to lock that same object  Now they might not be aware that you re locking on this internally  so this may cause problems  possibly a deadlock   In addition to this  it s also bad practice  because it s locking  too much   For example  you might have a member variable of List lt int gt   and the only thing you actually need to lock is that member variable  If you lock the entire object in your functions  then other things which call those functions will be blocked waiting for the lock  If those functions don t need to access the member list  you ll be causing other code to wait and slow down your application for no reason at all

User · Answer

Please refer to the following link which explains why lock  this  is not a good idea   https   docs microsoft com en-us dotnet standard threading managed-threading-best-practices  So the solution is to add a private object  for example  lockObject to the class and place the code region inside the lock statement as shown below   lock  lockObject

User · Answer

Take a look at the MSDN Topic Thread Synchronization  C  Programming Guide      Generally  it is best to avoid locking   on a public type  or on object   instances beyond the control of your   application  For example  lock this    can be problematic if the instance can   be accessed publicly  because code   beyond your control may lock on the   object as well  This could create   deadlock situations where two or more   threads wait for the release of the   same object  Locking on a public   data type  as opposed to an object    can cause problems for the same   reason  Locking on literal strings is   especially risky because literal   strings are interned by the common   language runtime  CLR   This means   that there is one instance of any   given string literal for the entire   program  the exact same object   represents the literal in all running   application domains  on all threads    As a result  a lock placed on a string   with the same contents anywhere in the   application process locks all   instances of that string in the   application  As a result  it is best   to lock a private or protected member   that is not interned  Some classes   provide members specifically for   locking  The Array type  for example    provides SyncRoot  Many collection   types provide a SyncRoot member as   well

User · Answer

Here s a much simpler illustration  taken from Question 34 here   why lock this  is bad and may result in deadlocks when consumer of your class also try to lock on the object  Below  only one of three thread can proceed  the other two are deadlocked    class SomeClass       public void SomeMethod int id                  lock this                          while true                                Console WriteLine  SomeClass SomeMethod      id                                    class Program       static void Main string   args                SomeClass o   new SomeClass             lock o                        for  int threadId   0  threadId  lt  3  threadId                                  Thread t   new Thread      gt                        o SomeMethod threadId                                               t Start                               Console WriteLine                 To work around  this guy used Thread TryMonitor  with timeout  instead of lock                Monitor TryEnter temp  millisecondsTimeout  ref lockWasTaken               if  lockWasTaken                                doAction                              else                               throw new Exception  Could not get lock                     https   blogs appbeat io post c-how-to-lock-without-deadlocks

User · Answer

Because any chunk of code that can see the instance of your class can also lock on that reference  You want to hide  encapsulate  your locking object so that only code that needs to reference it can reference it  The keyword this refers to the current class instance  so any number of things could have reference to it and could use it to do thread synchronization   To be clear  this is bad because some other chunk of code could use the class instance to lock  and might prevent your code from obtaining a timely lock or could create other thread sync problems  Best case  nothing else uses a reference to your class to lock  Middle case  something uses a reference to your class to do locks and it causes performance problems  Worst case  something uses a reference of your class to do locks and it causes really bad  really subtle  really hard-to-debug problems

[c#] Why is lock(this) {...} bad?

Examples related to c#

Examples related to multithreading

Examples related to locking