0

I was testing Monitor with lock taken overload as following.

public static void Enter(object obj, ref bool lockTaken);

I have created following sample for this however not sure what's the easy fix.

class TestMonitor
    {
        int num1 = 0;
        int num2 = 0;
        Random rnd = new Random();
        private static object myLock = new object();
        bool isLockTaken = false;
        public void DoDivide()
        {
            try
            {
                Monitor.Enter(myLock, ref isLockTaken); //t2 fails here.
                {
                    for (int i = 0; i < 5; i++)
                    {
                        num1 = rnd.Next(1, 5);
                        num2 = rnd.Next(1, 5);
                        Console.WriteLine(num1 / num2);
                        num1 = 0;
                        num2 = 0;
                    }
                }
            }
            finally
            {
                if (isLockTaken)  { Monitor.Exit(myLock); }
            }
         }
    }

 class Program
    {        
        static void Main(string[] args)
        {
            Console.Title = "Monitor Demo";
            TestMonitor testThreadSafe = new TestMonitor();
            Thread t1 = new Thread(testThreadSafe.DoDivide);
            Thread t2 = new Thread(testThreadSafe.DoDivide);            
            t1.Start();
            t2.Start();  
        }
    }

I am getting error when 2nd thread (t2) try to access Monitor.Enter(myLock, ref isLockTaken); where it find that isLockTaken is true although it expects isLockTaken as false. isLockTaken is true made by 1st thread (t1) because it has obtained the lock. I kind of understood the issue but can someone points me towards easy fix so that both thread can work without issues.

Prakash Tripathi
  • 469
  • 6
  • 12
  • Thnx Gediminas Masaitis for the response. I know we can use lock keyword which automatically handles. However I was trying to play with Monitor overload. – Prakash Tripathi Mar 13 '16 at 13:54
  • I just tested that when I put bool isLockTaken = false; inside DoDivide, it works without any issue. However will look for some other comments if I am missing anything here. – Prakash Tripathi Mar 13 '16 at 13:56

2 Answers2

4

An easy fix would be to not use the isLockTaken boolean at all:

try
{
    Monitor.Enter(myLock);
    // ...
}
finally
{
    Monitor.Exit(myLock);
}

Or even better, to use a lock statement:

lock(myLock)
{
    // ...
}

However if you want to use the overload with ref bool lockTaken, the MSDN page states:

The input must be false.

The simple way to do this would be to move your isLockTaken field into a local method variable. Then, since each thread gets it's own stack, it would get it's own copy of the isLockTaken variable.

bool isLockTaken = false;
try
{
    Monitor.Enter(myLock, ref isLockTaken);
    // ...
}
finally
{
    if(isLockTaken)
    {
        Monitor.Exit(myLock);
    }
}
Gediminas Masaitis
  • 3,172
  • 14
  • 35
  • Thnx Gediminas Masaitis. It fixes the issue. – Prakash Tripathi Mar 13 '16 at 14:28
  • +1 for lock(myLock), as it's probably better and easier to understand depending on the situation. Read more at http://stackoverflow.com/questions/2837070/lock-statement-vs-monitor-enter-method – Crypth Mar 13 '16 at 14:31
2

From Microsoft documentation:

lockTaken Type: System.Boolean

The result of the attempt to acquire the lock, passed by reference. The input must be false. The output is true if the lock is acquired; otherwise, the output is false. The output is set even if an exception occurs during the attempt to acquire the lock.

Since you're declaring isLockTaken in the class, it'll become true and keep being true. It should go into the dodivide function.

Crypth
  • 1,576
  • 18
  • 32