GOTCHA 57 Locking on globally visible objects is too sweeping


GOTCHA #57 Locking on globally visible objects is too sweeping

In Gotcha #56, "Calling Type.GetType() may not return what you expect," I discussed options for synchronizing threads. While the last solution presented there seems to work for that specific example, there is still a problem. Locking the type metadata to define a critical section is much too sweeping. If all static/Shared methods synchronize on the type metadata, then all access to the class methods is one-at-a-time, even if it doesn't need to be. This certainly provides thread safety, but sacrifices concurrency in ways that are neither necessary nor desirable in most cases. Consider Example 7-12.

Example 7-12. Consequence of locking the Type metadata

C# (SynchWithIntent)

 //Bacteria.cs using System; using System.Threading; namespace SynchOnType {     public class Bacteria     {         private static int bacteriaCount;         private static void IncreaseCount()         {             Console.WriteLine(                 "IncreaseCount called by {0} at {1}",                 AppDomain.GetCurrentThreadId(),                 DateTime.Now.ToLongTimeString());             bacteriaCount++;             Thread.Sleep(2000);             // Used for illustration purpose         }         public Bacteria()         {             lock(typeof(Bacteria))             {                 IncreaseCount();             }         }     } } //Test.cs using System; using System.Threading; namespace SynchOnType {     class Test     {         private static void Worker()         {             Console.WriteLine("In thread {0}",                 AppDomain.GetCurrentThreadId());             Bacteria aBacteria = new Bacteria();         }         [STAThread]         static void Main(string[] args)         {             Thread thread1 = new Thread(new ThreadStart(Worker));             Thread thread2 = new Thread(new ThreadStart(Worker));             lock(typeof(Bacteria))             {                 Console.WriteLine("Starting threads at {0}",                     DateTime.Now.ToLongTimeString());                 thread1.Start();                 thread2.Start();                 Thread.Sleep(3000);             }         }     } } 

VB.NET (SynchWithIntent)

 Imports System.Threading 'Bacteria.vb Public Class Bacteria     Private Shared bacteriaCount As Integer     Private Shared Sub IncreaseCount()         Console.WriteLine( _          "IncreaseCount called by {0} at {1}", _          AppDomain.GetCurrentThreadId(), _          DateTime.Now.ToLongTimeString())         bacteriaCount += 1         Thread.Sleep(2000)         ' Used for illustration purpose     End Sub     Public Sub New()         SyncLock GetType(Bacteria)             IncreaseCount()         End SyncLock     End Sub End Class 'Test.vb Imports System.Threading Module Test     Private Sub Worker()         Console.WriteLine("In thread {0}", _             AppDomain.GetCurrentThreadId())         Dim aBacteria As New Bacteria     End Sub     Public Sub Main()         Dim thread1 As New Thread(AddressOf Worker)         Dim thread2 As New Thread(AddressOf Worker)         SyncLock (GetType(Bacteria))             Console.WriteLine("Starting threads at {0}", _                  DateTime.Now.ToLongTimeString())             thread1.Start()             thread2.Start()             Thread.Sleep(3000)         End SyncLock     End Sub End Module 

When you execute the above program, since the Main() method holds a lock on the Bacteria type for 3 seconds, the creation of the Bacteria objects is delayed until Main() lets go of that lock. One reason why a client of the Bacteria class might hold a lock on its metadata is so it can call multiple static/Shared methods in a thread-safe way. For whatever reason, if Main() or any other method in the application grabs a lock on the Bacteria metadata, the execution of the constructors (and all other methods) is delayed. This is shown in the output in Figure 7-13.

Figure 7-13. Output from Example 7-12


The problem here is that Bacteria's constructor claims a lock on the Bacteria type in order to synchronize the execution of its static/Shared method. The static/Shared method and the implementation of the constructor are purely private to the class. However, you rely on a publicly visible object (the metadata of Bacteria) to synchronize. You can lock much more locally and precisely. Let's look at a modified implementation in Example 7-13.

Example 7-13. Synchronizing locally and precisely

C# (SynchWithIntent)

 //Bacteria.cs using System; namespace SynchOnType {     public class Bacteria     {         private static int bacteriaCount;         private static object theIncrementCountLock = new Object();         private static void IncreaseCount()         {             Console.WriteLine(                 "IncreaseCount called by {0} at {1}",                 AppDomain.GetCurrentThreadId(),                 DateTime.Now.ToLongTimeString());             bacteriaCount++;             System.Threading.Thread.Sleep(2000);             // Used for illustration purpose         }         public Bacteria()         {             lock(theIncrementCountLock)             {                 IncreaseCount();             }         }     } } 

VB.NET (SynchWithIntent)

 Imports System.Threading 'Bacteria.vb Public Class Bacteria     Private Shared bacteriaCount As Integer     Private Shared theIncrementCountLock As New Object     Private Shared Sub IncreaseCount()         Console.WriteLine( _          "IncreaseCount called by {0} at {1}", _          AppDomain.GetCurrentThreadId(), _          DateTime.Now.ToLongTimeString())         bacteriaCount += 1         Thread.Sleep(2000)         ' Used for illustration purpose     End Sub     Public Sub New()         SyncLock theIncrementCountLock             IncreaseCount()         End SyncLock     End Sub End Class 

In this case, you have created a dummy object (theIncrementCountLock refers to it) within the Bacteria class. In the constructor, you lock this object. Note that the dummy lock-facilitator object is private, so no method outside the class can claim a lock with it.

There are two advantages to this approach. One, code in other classes can't affect the concurrency of the constructor, because they can't see theIncrementCountLock. Second, suppose you have two different tasks, call them Task A and Task B. Each of these tasks must be executed one-at-a-time. But if Task A doesn't access any resources that Task B needs, and Task B touches none of Task A's resources, there is no problem with their running concurrently. You can realize this most effectively using two private dummy-lock objects. The output after this change is shown in Figure 7-14.

Figure 7-14. Output after change in Example 7-13


For a great discussion on possible deadlock consequences, refer to the article "Don't Lock Type Objects!" in the "on the web" section of the Appendix.

IN A NUTSHELL

Do not synchronize within your class on publicly visible objects. Synchronize precisely by synchronizing locally.

SEE ALSO

Gotcha #20, "Singleton isn't guaranteed process-wide," Gotcha #56, "Calling Type.GetType() may not return what you expect," Gotcha #62, "Accessing WinForm controls from arbitrary threads is dangerous," and Gotcha #64, "Raising events lacks thread-safety."



    .NET Gotachas
    .NET Gotachas
    ISBN: N/A
    EAN: N/A
    Year: 2005
    Pages: 126

    flylib.com © 2008-2017.
    If you may any questions please contact us: flylib@qtcs.net