Click here to Skip to main content
12,752,876 members (39,592 online)
Click here to Skip to main content
Add your own
alternative version

Tagged as

Stats

4.6K views
1 bookmarked
Posted 29 May 2010

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

, 30 May 2010 CPOL
Rate this:
Please Sign up or sign in to vote.
One of bad practices a programmer usually fall in.
Source- 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:



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(new object());
    }

    foreach (Thread t in arr)
        t.Join();
}

private static void ThreadProc(object obj)
{
    lock (obj)
    {
        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,      2
Thread #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!

License

This article, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Share

About the Author

Mohammad Elsheimy
Technical Writer Just Like a Magic
Egypt Egypt
Independent technical writer from Egypt born in 1991.

I am a student of The Holy Quranic Sciences Institute. There, we study the Holy Quranic sciences and Islamic legislation.

I currently run two sites:

You may also be interested in...

Comments and Discussions

 
GeneralReason for my vote of 1 What's the point of making this arti... Pin
usernameCasper11-Jan-11 8:30
memberusernameCasper11-Jan-11 8:30 

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.

Permalink | Advertise | Privacy | Terms of Use | Mobile
Web02 | 2.8.170217.1 | Last Updated 30 May 2010
Article Copyright 2010 by Mohammad Elsheimy
Everything else Copyright © CodeProject, 1999-2017
Layout: fixed | fluid