Are there any loss in this alternative way of using c# Mutex class?

Question

I have an application (I didn't write this) that is using Mutex like this:

static void Main(string[] args)
{
    Mutex mutex = null;

    try
    {
        mutex = Mutex.OpenExisting("SINGLEINSTANCE");

        if (mutex != null)
        {
            return;
        }
    }
    catch (WaitHandleCannotBeOpenedException ex)
    {
        mutex = new Mutex(true, "SINGLEINSTANCE");
    }

    // critical section here
}

But I know that the correct way is this:

private readonly Mutex m = new Mutex("SINGLEINSTANCE");
static void Main(string[] args) {
    m.WaitOne();
    try {
        /* critical code */
    }
    finally {
        m.ReleaseMutex();
    }
}

This is used because only one process of this application can run at the same time. It is a console application that do some asynchronous job for a web application.

This code is in production, I don't wanna change it, unless there are some big problem with this code... Are there?

If it's in production and it's not broken then why do you want to change or fix it? — CodingYoshi, Jan 17 '17 at 18:48
Your "Correct way" has a different behavior. the old way closes the program if the mutex exists, your new way just waits for the mutex to be free. However, the old way still has a race condition. Also, the Mutex constructor takes a `bool` in as a paramter, the value true or false on that bool greatly affects how your code would work for this use case. — Scott Chamberlain, Jan 17 '17 at 18:49

Scott Chamberlain · Accepted Answer · 2017-01-17T19:16:11.037

The old code has a race condition where two processes could try to start at the same time, the 2nd one to start will throw a exception and crash on the new Mutex(true, "SINGLEINSTANCE"); line. If the mutex already existed and you don't have the race condition the program will quit gracefully and never execute the critical section.

Your new code can't compile because you did not use a valid constructor for Mutex, however if you had passed in false your code would wait for the previous mutex to be released then it would continue on with the critical section, never exiting the program early.

The "Correct" way would be to combine the two methods and try and create the mutex and see if you are the owner of it using the overload with the out parameter.

static void Main(string[] args)
{
    bool taken;
    using(Mutex mutex = new Mutex(true, "SINGLEINSTANCE", out taken))
    {
        if(!taken)
            return;
        try
        {
            // critical section here
        }
        finally
        {
            mutex.ReleaseMutex();
        }
    }
}

Are there any loss in this alternative way of using c# Mutex class?

1 Answers1