3

I'm not exactly sure how to address this issue. I have a mutex that is declared as such:

public class MyNamedLock
{
    private Mutex mtx;
    private string _strLkName;

    public MyNamedLock(string strLockName)
    {
        _strLkName = strLockName;
        //...

        mtx = new Mutex(false, _strLkName, out bCreatedNew, mSec);
    }

    public bool enterLockWithTimeout(int nmsWait = 30 * 1000)
    {
        _nmsWaitLock = nmsWait;

        //Wait
        return mtx.WaitOne(nmsWait);
    }

    public void leaveLock()
    {
        _nmsWaitLock = 0;

        //Release it
        mtx.ReleaseMutex();
    }
}

Then it is used in an ASP.NET page as such:

public class MyClass
{
    private MyNamedLock gl;

    public MyClass()
    {
        gl = new MyNamedLock("lock name");

    }


    public void funct()
    {
        try
        {
            //Enter lock
            if (gl.enterLockWithTimeout())
            {
                //Do work
            }
            else
                throw new Exception("Failed to enter lock");
        }
        finally
        {
            //Leave lock
            gl.leaveLock();
        }
    }
}

This code doesn't give me any trouble in my dev environment but in the production it sometimes throws this exception:

Object synchronization method was called from an unsynchronized block of code.

The description is kinda vague, but just doing the trace I found out that the exception is raised at the mtx.ReleaseMutex(); part. What does it mean and how to fix it?

ahmd0
  • 16,633
  • 33
  • 137
  • 233
  • Among other issues, note that the `Mutex` class implements `IDisposable`. This means that any class that holds a reference to an instance of your class should also implement `IDisposable`. If your `MyNamedLock` implemented `IDisposable`, then you'd be able to take advantage of the `using` block in your code. – John Saunders Mar 14 '13 at 23:40
  • @JohnSaunders: So how shall I change the code above to do that? – ahmd0 Mar 14 '13 at 23:43

2 Answers2

1

In your finally block you're releasing the mutex regardless of whether you actually acquired it in your try block.

In

try
{
    //Enter lock
    if (gl.enterLockWithTimeout())
    {
        //Do work
    }
    else throw new Exception("Failed to enter lock");
}
finally
{
    //Leave lock
    gl.leaveLock();
}

if gl.enterLockWithTimeout returns false, you will throw an exception but then try to release the lock in the finally block.

Lee
  • 142,018
  • 20
  • 234
  • 287
1

You have some issues on your class, and on the way you use it.

  1. You must release the mutex only if you have previous locked (and this is your error)
  2. You need to Close and Dispose your opened mutex
  3. Also is better to create it just before you going to use it and not when you create you class MyClass.

So I suggest at first look to change your class as:

public class MyNamedLock
{
    private Mutex mtx = null;
    private string _strLkName;

    // to know if finally we get lock
    bool cNeedToBeRelease = false;

    public MyNamedLock(string strLockName)
    {
        _strLkName = strLockName;
        //...

        mtx = new Mutex(false, _strLkName, out bCreatedNew, mSec);
    }

    public bool enterLockWithTimeout(int nmsWait = 30 * 1000)
    {
        _nmsWaitLock = nmsWait;
        bool cLock = false;
        try
        {
            cLock = mtx.WaitOne(nmsWait, false);
            cNeedToBeRelease = cLock;    
        }
        catch (AbandonedMutexException)
        {
            // http://stackoverflow.com/questions/654166/wanted-cross-process-synch-that-doesnt-suffer-from-abandonedmutexexception
            // http://msdn.microsoft.com/en-us/library/system.threading.abandonedmutexexception.aspx
            cNeedToBeRelease = true;
        }
        catch (Exception x)
        {
            // log the error
            Debug.Fail("Check the reason of fail:" + x.ToString());
        }            

        return cLock;        
    }

    public void leaveLock()
    {
        _nmsWaitLock = 0;

        if (mtx != null)
        {
            if (cNeedToBeRelease)
            {
                try
                {
                    mtx.ReleaseMutex();
                    cNeedToBeRelease = false;    
                }
                catch (Exception x)
                {
                    Debug.Fail("Check the reason of fail:" + x.ToString());
                }
            }

            mtx.Close();
            mtx.Dispose();
            mtx = null;
        }
    }
}

This the way you must call that class:

public class MyClass
{
    public MyClass()
    {
    }


    public void funct()
    {
        var gl = new MyNamedLock("lock name");
        try
        {
            //Enter lock
            if (gl.enterLockWithTimeout())
            {
                //Do work
            }
            else
                throw new Exception("Failed to enter lock");
        }
        finally
        {
            //Leave lock
            gl.leaveLock();
        }
    }
}
Aristos
  • 66,005
  • 16
  • 114
  • 150
  • Thanks. One question though. Can I still proceed with entering the locked section if I get `AbandonedMutexException` on the mutex? – ahmd0 Mar 15 '13 at 03:50
  • @ahmd0 Yes you can, thats why I have the cLock=true. – Aristos Mar 15 '13 at 05:30
  • Yes, I noticed that. While doing my own search I came across this thread. Can it be the way I create the Mutex? What's bugging me is why do I get `unsynchronized block of code` exception? Here's the thread I referred to: http://stackoverflow.com/questions/7605077/object-synchronization-method-was-called-from-an-unsynchronized-block-of-code – ahmd0 Mar 15 '13 at 05:34
  • @ahmd0 Yes will be probably the same, you try to release it with out been actually locked before. What you need to focus is that this is a resource that you need to open, use, close, and dispose. And release it only if you actually lock it. This way as I have type it you can do your work with out other many issue, some small changes on the class. (this code is from actually proved working web pages for the more than 3 years) – Aristos Mar 15 '13 at 09:35
  • After the further review I don't think you're handling the `AbandonedMutexException` right. Shouldn't you at least call `WaitOne` again before returning `true`? – ahmd0 Mar 17 '13 at 04:31
  • @ahmd0 read that `The exception that is thrown when one thread acquires a Mutex object that another thread has abandoned by exiting without releasing it.` Maybe I have right at the first place.... – Aristos Mar 17 '13 at 10:38