.NET Tutorials, Forums, Interview Questions And Answers
Welcome :Guest
 
Sign In
Register
 
Win Surprise Gifts!!!
Congratulations!!!


Top 5 Contributors of the Month
david stephan

Home >> Articles >> Architecture/Pattern >> Post New Resource Bookmark and Share   

 Subscribe to Articles

Bad Practices: Locking on Non-shared Objects in Multi-threaded Applications

Posted By:Mohammad Elsheimy       Posted Date: May 02, 2010    Points: 25    Category: Architecture/Pattern    URL: http://www.dotnetspark.com  

Bad Practices: Locking on Non-shared Objects in Multi-threaded Applications. In this article we will see one of the bad practices developers always do.
 

This article was previously published in my blog, Just Like a Magic.

Actually, I was having a problem synchronizing threads calling a function. If we could regenerate the bug, we would end up with code like this:

private static object _lockThis = new object();
static void Main()
{
    Thread[] arr = new Thread[10];
    for (int i = 0; i < 10; i++)
    {
        arr[i] = new Thread(ThreadProc);
        arr[i].IsBackground = true;
        arr[i].Start();
    }
    foreach (Thread t in arr)
        t.Join();
}
private static void ThreadProc(object obj)
{
    lock (_lockThis)
    {
        int i = 0;
        while (i < 10)
        {
            Thread.Sleep(1);
            Console.WriteLine("Thread #{0},\t{1}",
                Thread.CurrentThread.ManagedThreadId, i++);
        }
    }
}


And when we execute this code the results would be like this:

Thread #4,
2 Thread #3,
3 Thread #5,
1 Thread #7,
0 Thread #5,
2Thread #6,
1 Thread #3,
4 Thread #4,
3 Thread #8,
1 Thread #9, 0

What is the problem with this code? It starts multiple threads and passes them a locking object and the object is locked successfully using the lock statement, so why threads overlap? Why the synchronization doesn't take effect?

Well, after a long period and after pushing a new thread in the MSDN forums (we are all developers do make silly mistakes, aih? :P), I come up with the solution and discovered the drawback of the code.

The problem was that each time we start off a thread we pass it a new locking object different from the others:

arr[i].Start(new object());

Therefore, every thread is locking on its own objects, so no thread synchronization take effect.

The solution is very easy, you should lock on a shared object; an object that is shared between all your threads accessing this block of code. Read more about thread synchronization here.

For example, we should change our code to the following:

private static object _lockThis = new object();
static void Main()
{
    Thread[] arr = new Thread[10];
    for (int i = 0; i < 10; i++)
    {
        arr[i] = new Thread(ThreadProc);
        arr[i].IsBackground = true;
        arr[i].Start();
    }
    foreach (Thread t in arr)
        t.Join();
}
private static void ThreadProc(object obj)
{
    lock (_lockThis)
    {
        int i = 0;
        while (i < 10)
        {
            Thread.Sleep(1);
            Console.WriteLine("Thread #{0},\t{1}",
                Thread.CurrentThread.ManagedThreadId, i++);
        }
    }
}


Finally, this was one of the bad practices and mistakes me (and many others too) fall in. Honestly, I would like to start my Bad Practices series :">. I would write about problems I came across and solutions I found for them. In addition, I'm thinking of starting the Questions series. I got a huge number of questions every week, if we could publish them I think it would be great for all.

Have a nice day!

 Subscribe to Articles

     

Further Readings:

Responses

No response found. Be the first to respond this post

Post Comment

You must Sign In To post reply
Find More Articles on C#, ASP.Net, Vb.Net, SQL Server and more Here

Hall of Fame    Twitter   Terms of Service    Privacy Policy    Contact Us    Archives   Tell A Friend