CODEWITHSUNDEEP
c#multithreadingdeadlock
Bose_geek
Bose_geek

Reputation: 498

Potential deadlock?

public class MyClass
{
  public void DoSomething()
  {
   Lock(this)
   {
        //Do Something.
   }
  }
}

public class AnotherClass
{
   MyClass myclass = new MyClass();
   Public void DoAnotherThing()
   {
     lock(myclass)
     {
        Myclass.DoSomething();
     }
   }
}

Will this create a deadlock ?

As per my understanding and as per the articles I have read – It will. Why ? – Whenever DoSomething() is called it will try to attain a lock and wait for lock(myclass) to be released and hence a deadlock. Please confirm my understanding(a little explanation is also requested) and correct me if I am wrong.

Upvotes: 0

Views: 289

Answers (2)

Matthew Watson
Matthew Watson

Reputation: 109567

I think what the articles you read were trying to tell you is that you shouldn't lock (this) because some other code might also try to lock the same object. This will only happen if two or more threads are involved.

Here's some sample code which demonstrates a deadlock problem. Try running it and look at the result. Then make the edit suggested on the lock (this) line and try it again.

The deadlock occurs because some code OUTSIDE the class is locking on the same instance of the class that the code INSIDE the class is using for a lock - and without careful documentation and visibility, that's a real possibility.

The moral of this story is that in general you should not lock on anything that is visible outside the class (unless you document carefully how to use the locking for that class), and you should NEVER lock (this).

using System;
using System.Threading.Tasks;

namespace Demo
{
    class MyClass
    {
        public void DoSomething()
        {
            Console.WriteLine("Attempting to enter the DoSomething lock.");

            lock (this) // Change to lock(_locker) to prevent the deadlock.
            {
                Console.WriteLine("In the DoSomething lock.");
            }
        }

        readonly object _locker = new object();
    }

    internal static class Program
    {
        static void Main(string[] args)
        {
            var myClass = new MyClass();

            lock (myClass)
            {
                var task = Task.Run(() => myClass.DoSomething());

                Console.WriteLine("Waiting for the task to complete.");

                if (!task.Wait(1000))
                    Console.WriteLine("ERROR: The task did not complete.");
                else
                    Console.WriteLine("Task completed.");

                Console.WriteLine("Press any key to continue...");
                Console.ReadKey();
            }
        }
    }
}

Upvotes: 1

Sneftel
Sneftel

Reputation: 41474

This cannot create a deadlock, because only one thread is involved. True, a lock on your MyClass object is requested when a lock on it is already held. But C# locks are "recursive", meaning that a single thread can hold multiple locks on the same object, with the result being the same as if it only held the outermost lock: The inner locks immediately succeed, and the object is locked until the last lock is released. (You don't truly understand how useful this is until you're forced to use a language which doesn't have it.)

But I agree with everyone above: lock(this) is bad juju.

Upvotes: 0

Related Questions