acquire multiple thread sync locks simultaneously

Question

Let's assume following code, which is used in similar way in my application:

//-------------------------------------
void UseAllResources ()
{
  bool bSuccess1 = false;
  bool bSuccess2 = false;
  try
  {
    bSuccess1 = Monitor::TryEnter (oResource1, msc_iTimeoutMonitor);
    if (!bSuccess1) return;
    bSuccess2 = Monitor::TryEnter (oResource2, msc_iTimeoutMonitor);
    if (!bSuccess2) return;

    // work on oResource1 and oResource2
  } finally {
    if (bSuccess2)
      Monitor::Exit (oResource2);
    if (bSuccess1)
      Monitor::Exit (oResource1);
  }
}

//-------------------------------------
void UseResource1 ()
{
  bool bSuccess = false;
  try {
    bSuccess = Monitor::TryEnter (oResource1, msc_iTimeoutMonitor);
    if (!bSuccess) return;

    // work on oResource1
  } finally {
    if (bSuccess) Monitor::Exit (oResource1);
  }
}

//-------------------------------------
void UseResource2 ()
{
  same like UseResource1(), but using oResource2
}

These functions may get called at any time by different threads.

It may happen that
(timeout is 500ms)
@t=0ms, thread B is executing UseResource2(), will take 400ms,
@t=100ms, thread Z is calling UseAllResources (), takes the lock on oResource1 and has to wait for the lock on oResource2,
@t=200ms, thread A is calling UseResource1() and has to wait for the lock on oResource1, which is taken by thread Z,
@t=400ms, thread B completes, thread Z takes the lock on oResource2 and starts work, will take 400ms,
@t=700ms, thread A times out, although it would have needed only 50ms and could have worked while thread Z was still waiting.

I rather want thread Z to fail, if at all, because the timeout should be an overall value for all locks.

Can I start acquisition of multiple locks simultaneously?

No, you can't acquire multiple locks all together (unless you introduce another outer lock to synchronize lock acquisition...bad, IMO). However multiple acquired locks are a good starting point for headaches and white debugging nights (because of easy deadlocks), can't you acquire and use only one resource at time? BTW you may want to use `msclr::lock` instead of _long syntax_. If resources are related then you'd **better to use one coarser lock than playing with two fine locks** (and investigate performance of `ReaderWriterLockSlim`) — Adriano Repetti, May 30 '16 at 08:01
@AdrianoRepetti: in some functions I need to have both locks because I shift data from one resource to another. And by using timeouts there shouldn't be any deadlocks. — Tobias Knauss, May 30 '16 at 08:11
true but if you can't acquire the lock you just...return without performing required operation. Is it really what you want in production? You're asking this question because timeout...well doesn't work as you wish. The point is...don't acquire simultaneous locks and move to a coarser (shared) one. Locking strategy is (IMO) too complex to be designed with a fictional example, what's your read pattern? Write pattern? Concurrent reads and writes? What's the performance hit to use a coarser lock? What's the benefit of finer locks? BTW if you have to you need to introduce a lock-acquisition lock... — Adriano Repetti, May 30 '16 at 08:42
@AdrianoRepetti I agree that a common lock makes sense. I will likely use it instead of single locks on each resource. — Tobias Knauss, May 30 '16 at 08:45
@AdrianoRepetti: No matter if I like Obama or not, but: yes I can. Found a nice solution. — Tobias Knauss, May 30 '16 at 16:02

Jeff Pang · Answer 1 · 2016-05-30T11:25:27.990

A solution may be to use ReaderWriterLockSlim class. The following code wraps a function (your work to be done), in the constructor. Alternatively you may move the function to the DoWork method to change how you access the resource.

LockedResource implmentation

class LockedResource
{
    public delegate void RefAction();

    ReaderWriterLockSlim resourceLock;

    public LockedResource()
    {
        //Warning: SupportsRecursion is risky, you should remove support for recursive whenever possible
        resourceLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
    }

    public bool DoWork(RefAction work, string threadname, int timeout = -1)
    {
        try
        {
            if (resourceLock.TryEnterWriteLock(timeout))
            {
                if (work != null)
                {
                    work();
                }
            }
            else
            {
                Console.WriteLine("Lock time out on thread {0}", threadname);
            }
        }
        finally
        {

            Console.WriteLine("{0} releasing resource", threadname);
            if(resourceLock.IsWriteLockHeld)
            {
                resourceLock.ExitWriteLock();
            }
        }

        return false;
    }
}

Sample Usage

static void Main(string[] args)
{
        object oResouce1 = "-";
        object oResouce2 = "-";

        LockedResource lock1 = new LockedResource();
        LockedResource lock2 = new LockedResource();

       //the event wait handles is not required, only used to block thread so that resource values can be printed out at the end of the program
        var h1 = new EventWaitHandle(false, EventResetMode.ManualReset);
        var h2 = new EventWaitHandle(false, EventResetMode.ManualReset);
        var h3 = new EventWaitHandle(false, EventResetMode.ManualReset);

        WaitHandle[] waitHandles = { h1, h2, h3 };

        var t1 = new Thread(() =>
        {
            lock1.DoWork(() =>
            {
                oResouce1 = "1";
                Console.WriteLine("Resource 1 set to 1");
            },"T1");

            h1.Set();
        });

        var t2 = new Thread(() =>
        {
            lock2.DoWork(() =>
            {
                oResouce2 = "2";
                Console.WriteLine("Resource 2 set to 2");
                Thread.Sleep(10000);

            }, "T2");
            h2.Set();
        });

        var t3 = new Thread(() =>
        {
            lock1.DoWork(() =>
            {
                lock2.DoWork(() =>
                {
                    oResouce1 = "3";
                    Console.WriteLine("Resource 1 set to 3");

                    oResouce2 = "3";
                    Console.WriteLine("Resource 2 set to 3");
                }, "T3", 1000);
                h3.Set();

            },  "T3");

        });
        t1.Start();
        t2.Start();
        t3.Start();


        WaitHandle.WaitAll(waitHandles);
        Console.WriteLine("Resource 1 is {0}", oResouce1);
        Console.WriteLine("Resource 2 is {0}", oResouce2);

        Console.ReadLine();
}

Output

Resource 1 set to 1
Resource 2 set to 2
T1 releasing resource
Lock time out on thread T3
T3 releasing resource
T3 releasing resource
T2 releasing resource
Resource 1 is 1
Resource 2 is 2

If I understand correctly, `t3` is not taking the resources 1 and 2 at the same time, but after each other. This is not what I intend. I want to perform work which simultaneously used 2 different resources. Here you seem to wrap each resource into a single lock and work consecutively. — Tobias Knauss, May 30 '16 at 09:08
@jeff LockRecursionPolicy.SupportsRecursion should be used with cum grano salis (IMO), re-entrant code is inherently dangerous. ReaderWriterLockSlim may be (it depends on OP's usage pattern) but it won't mitigate (again, it depends) double-locking issues. — Adriano Repetti, May 30 '16 at 09:11
@AdrianoRepetti Thanks for the heads up, I am aware of this issue but I agree and my example should have come with a warning. Thanks. — Jeff Pang, May 30 '16 at 09:51
@TobiasKnauss, ok, I am still not certain if am clear on your use case, but I will post another variation shortly, hopefully that will be what you are looking for. — Jeff Pang, May 30 '16 at 09:52
@JeffPang: don't put too much work into it for me. If the solution is too complex, I'm likely not gonna use it and use a single lock instead. — Tobias Knauss, May 30 '16 at 09:53
@JeffPang: now, after your changes, I don't see any difference to mine. `t3`is taking the locks one after another. Sure, the behaviour of your code very likely is correct, just the timing probably is not as intended. I will add a timing sample to my question. However, thanks for your suggestions. — Tobias Knauss, May 30 '16 at 11:31

Tobias Knauss · Accepted Answer · 2016-05-30T16:07:18.407

EDIT:
tl;dr
It works. The out-of-the-box solution is at the end of this answer.
/EDIT

While adding a timing sample to my question, I had the idea for a solution:

My target is to get the locks only when all of them are free (implemented below), but it could easily be changed to keeping the locks and only returning the received locks on timeout.

This part could further be moved to a static helper function which receives an array of objects (or mutexes) to lock and the timeout.
EDIT:
Done, see end of the answer.

//-------------------------------------
// using direct implementation
//-------------------------------------
void UseAllResources2 ()
{
  bool bSuccess1 = false;
  bool bSuccess2 = false;
  try
  {
    Console::WriteLine (DateTime::Now.ToString("ss.fff") + " "__FUNCTION__"() start locking 1 and 2");
    DateTime tStart = DateTime::Now;
    bool bSuccess = false;
    do
    {
      bSuccess1 = Monitor::TryEnter (oResource1);
      bSuccess2 = Monitor::TryEnter (oResource2);
      bSuccess = bSuccess1 && bSuccess2;
      if (!bSuccess)
      {
        if (bSuccess1) Monitor::Exit (oResource1);
        if (bSuccess2) Monitor::Exit (oResource2);
        Thread::Sleep(10);
      }
    }
    while (!bSuccess && (DateTime::Now - tStart).TotalMilliseconds < msc_iTimeoutMonitor);
    if (!bSuccess)
    {
      Console::WriteLine (DateTime::Now.ToString("ss.fff") + " "__FUNCTION__"() monitor timeout");
      return;
    }

    Console::WriteLine (DateTime::Now.ToString("ss.fff") + " "__FUNCTION__"() start work");
    Thread::Sleep (400);
    Console::WriteLine (DateTime::Now.ToString("ss.fff") + " "__FUNCTION__"() finish work");
  } finally {
    if (bSuccess2)
      Monitor::Exit (oResource2);
    if (bSuccess1)
      Monitor::Exit (oResource1);
  }
}

//-------------------------------------
// using Out-Of-Box solution
//-------------------------------------
static void UseAllResources3 ()
{
  bool bSuccess = false;
  try
  {
    Console::WriteLine (DateTime::Now.ToString("ss.fff") + " "__FUNCTION__"() start locking 1 and 2");
    bSuccess = MonitorTryEnter (gcnew array<Object^>{oResource1, oResource2}, 500, 10, false);
    if (!bSuccess)
    {
      Console::WriteLine (DateTime::Now.ToString("ss.fff") + " "__FUNCTION__"() monitor timeout");
      return;
    }

    Console::WriteLine (DateTime::Now.ToString("ss.fff") + " "__FUNCTION__"() start work");
    Thread::Sleep (400);
    Console::WriteLine (DateTime::Now.ToString("ss.fff") + " "__FUNCTION__"() finish work");
  } finally {
    if (bSuccess)
    {
      Monitor::Exit (oResource2);
      Monitor::Exit (oResource1);
    }
  }
}

my main() for test:

int main()
{
// first run is for the CLR to load everything
  Thread^ oThreadA = gcnew Thread (gcnew ThreadStart (&UseResource1));
  Thread^ oThreadB = gcnew Thread (gcnew ThreadStart (&UseResource2));
 // Thread^ oThreadZ = gcnew Thread (gcnew ThreadStart (&UseAllResources));
 // Thread^ oThreadZ = gcnew Thread (gcnew ThreadStart (&UseAllResources2));
  Thread^ oThreadZ = gcnew Thread (gcnew ThreadStart (&UseAllResources3));

  oThreadB->Start();
  Thread::Sleep(100);
  oThreadZ->Start();
  Thread::Sleep(100);
  oThreadA->Start();

  Thread::Sleep (2000);
  Console::WriteLine();

// now that all code is JIT compiled, the timestamps are correct.
// Logs below are from this 2nd run.
  oThreadA = gcnew Thread (gcnew ThreadStart (&UseResource1));
  oThreadB = gcnew Thread (gcnew ThreadStart (&UseResource2));
 // oThreadZ = gcnew Thread (gcnew ThreadStart (&UseAllResources));
 // oThreadZ = gcnew Thread (gcnew ThreadStart (&UseAllResources2));
  oThreadZ = gcnew Thread (gcnew ThreadStart (&UseAllResources3));

  oThreadB->Start();
  Thread::Sleep(100);
  oThreadZ->Start();
  Thread::Sleep(100);
  oThreadA->Start();

  Thread::Sleep (2000);
}

output UseAllResources(): (from question)
01.503 UseResource2() start locking
01.503 UseResource2() start work
01.604 UseAllResources() start locking 1
01.604 UseAllResources() start locking 2
01.707 UseResource1() start locking
01.903 UseResource2() finish work
01.903 UseAllResources() start work
02.211 UseResource1() monitor timeout
02.303 UseAllResources() finish work

output UseAllResources2(): (1st solution, direct implementation)
42.002 UseResource2() start locking
42.002 UseResource2() start work
42.103 UseAllResources2() start locking 1 and 2
42.206 UseResource1() start locking
42.206 UseResource1() start work
42.256 UseResource1() finish work
42.402 UseResource2() finish work
42.427 UseAllResources2() start work
42.827 UseAllResources2() finish work

output UseAllResources3(keepLocks=false): (2nd solution, out-of-box implementation)
16.392 UseResource2() start locking
16.393 UseResource2() start work
16.494 UseAllResources3() start locking 1 and 2
16.595 UseResource1() start locking
16.597 UseResource1() start work
16.647 UseResource1() finish work
16.793 UseResource2() finish work
16.818 UseAllResources3() start work
17.218 UseAllResources3() finish work
// same as previous, as expected.

output UseAllResources3(keepLocks=true): (2nd solution, out-of-box implementation)
31.965 UseResource2() start locking
31.965 UseResource2() start work
32.068 UseAllResources3() start locking 1 and 2
32.169 UseResource1() start locking
32.365 UseResource2() finish work
32.390 UseAllResources3() start work
32.672 UseResource1() monitor timeout
32.790 UseAllResources3() finish work
// timeout on thread A, as expected.

WORKS! :-)

tl;dr
Here's the out-of-box solution:

//----------------------------------------------------------------------------
// MonitorTryEnter
//----------------------------------------------------------------------------
bool MonitorTryEnter (array<Object^>^ i_aoObject, int i_iTimeout, int i_iSleep, bool i_bKeepLocks)
{
  if (!i_aoObject)
    return false;
  if (i_iSleep < 0)
    i_iSleep = 10;

  List<Object^>^ listObject = gcnew List<Object^>;
  for (int ixCnt = 0; ixCnt < i_aoObject->Length; ixCnt++)
    if (i_aoObject[ixCnt])
      listObject->Add (i_aoObject[ixCnt]);
  if (listObject->Count <= 0)
    return false;
  array<bool>^ abSuccess = gcnew array<bool>(listObject->Count);

  DateTime tStart = DateTime::Now;
  bool bSuccess = true;
  do
  {
    bSuccess = true;
    if (!i_bKeepLocks)
      abSuccess = gcnew array<bool>(listObject->Count);

    for (int ixCnt = 0; ixCnt < listObject->Count; ixCnt++)
    {
      if (!abSuccess[ixCnt])
        abSuccess[ixCnt] = Monitor::TryEnter (listObject[ixCnt]);
      bSuccess = bSuccess && abSuccess[ixCnt];
      if (!bSuccess)
        break;
    }

    if (!bSuccess)
    {
      if (!i_bKeepLocks)
      {
        for (int ixCnt = 0; ixCnt < listObject->Count; ixCnt++)
        {
          if (abSuccess[ixCnt])
          {
            Monitor::Exit (listObject[ixCnt]);
            abSuccess[ixCnt] = false;
          }
        }
      }
      Thread::Sleep(i_iSleep);
    }
  }
  while (!bSuccess && (DateTime::Now - tStart).TotalMilliseconds < i_iTimeout);

  if (!bSuccess)
  {
    for (int ixCnt = 0; ixCnt < listObject->Count; ixCnt++)
      if (abSuccess[ixCnt])
        Monitor::Exit (listObject[ixCnt]);
  }

  return bSuccess;
}

acquire multiple thread sync locks simultaneously

2 Answers2