CODEWITHSUNDEEP
c#synchronizationmutexinterprocess
lmcarreiro
lmcarreiro

Reputation: 5772

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

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?

Upvotes: 0

Views: 126

Answers (1)

Scott Chamberlain
Scott Chamberlain

Reputation: 127543

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();
        }
    }
}

Upvotes: 2

Related Questions