C# · Dotnet

Careful with locking objects

Dear Reader,

Today i was just going through some product code at work. There was this code which used multithreading concepts to do some work. At first it just went through the code without worrying much about its pitfalls. I didn’t even think there was one (how stupid of me?). But at some point down further some things started ringing some alarms in my brain. So at that point i went back to the pitfall code where my brain started give some alarms.

At this point i did not find any cranky things in the code, some thing i over saw i guess. So i started debugging because i got curious or perhaps my alarmed brain wasnt able to see the exact problems. So upon starting to debug the code step by step i suddenly realised reading some thing about locking object in Jeffrey ritchers book.

Thanks to him and my brain, to just pop that picture at that instant. I stopped debugging and began to analyze the code. Below is that production code though it’s not completely shown for obvious reasons:

static object lockObj = new object();

static void Main(string[] args)
{
    LockObject();
    try
    {
        //Some code
    }
    catch(/*Specific type*/)
    {
        //Some state recovery code
    }
    finally
    {
        Monitor.Exit(lockObj);
    }
}

public static void LockObject()
{
  Monitor.Enter(lockObj);
  //Some code here..
}

First the problem with the code is that lock entering method is called outside Try block, which could cause an exceptiom before or after obtaining the lock. This could result in ending up locking the object forever. Which is very problematic.

Second issue is that the Monitor.Lock API could throw some weird exceptions internally: this statement i recalled from Jeffreys book. So i began to go a bit depth into this API via ILDASM tool. I found that it internally uses many other API’s to do the job. First API was that it gets a resource string via Environment.GetResourceString(..), this API uses a resource file to get the data, now this can cause a failure right? That means Enter() API can cause a failure thus locking the object forever.

Hence, the best is be a bit careful here and use alternative APIs provided by the framework itself. As jeffrey suggests, use Monitor.TryEnter(..) API rather than just plain old. This will decrease the possiblity of the issue. Then put the statement inside Try block. So that there is always a cleanup taken care off by finally, thus fool proffing more than before.

Thats all my friend. Thanks for reading and thanks to Jeffrey.

Happy Coding,
Zen 🙂

EDIT: Further to my analysis on the same code, i once encountered an exception being thrown by Exit(..) API, this is because synchronization was not done over the locking object; in other words Enter API call did not succeed. Hence exiting on the same object will throw away exception. It is recommended to use TryEnter API via which you can make sure the object was really locked before even calling Exit API.

Advertisements

Leave a Reply

Please log in using one of these methods to post your comment:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s