ASP.NET lock thread method

Question

Hello,

I'm developing an ASP.NET forms webapplication using C#. I have a method which creates a new Order for a customer. It looks similar to this;

    private string CreateOrder(string userName) {
        // Fetch current order
        Order order = FetchOrder(userName);
        if (order.OrderId == 0) {
            // Has no order yet, create a new one
            order.OrderNumber = Utility.GenerateOrderNumber();
            order.Save();
        }
        return order;
    }

The problem here is, it is possible that 1 customer in two requests (threads) could cause this method to be called twice while another thread is also inside this method. This can cause two orders to be created.

How can I properly lock this method, so it can only be executed by one thread at a time per customer?

I tried;

    Mutex mutex = null;
    private string CreateOrder(string userName) {
        if (mutex == null) {
            mutex = new Mutex(true, userName);
        }
        mutex.WaitOne();
        // Code from above
        mutex.ReleaseMutex();
        mutex = null;
        return order;
    }

This works, but on some occasions it hangs on WaitOne and I don't know why. Is there an error, or should I use another method to lock?

Thanks

Answer 1

You should always try finally when releasing mutex. Also, make sure that the key is correct(userName)

Mutex mutex = null;
private string CreateOrder(string userName) {
    mutex = mutex ?? new Mutex(true, userName);
    mutex.WaitOne();
    try{
    // Code from above
    }finally{
    mutex.ReleaseMutex();
    }
    mutex = null;
    return order;
}

Answer 2

Unless I'm missing something, can't you just use a regular lock?

private object _locker = new object();

private string CreateOrder(string userName)
{
    lock(_locker)
    {
        // Fetch current order
        Order order = FetchOrder(userName);
        if (order.OrderId == 0)
        {
            // Has no order yet, create a new one
            order.OrderNumber = Utility.GenerateOrderNumber();
            order.Save();
        }
        return order;
    }
}

Answer 3

In your code, you are creating the mutex lazily. This leads to race conditions.
E.g. it can happen that the mutex is only partially constructed when you call WaitOne() from another thread.
It can also happen that you create two mutex instances.
etc...

You can avoid this by creating the instance eagerly - i.e. as in Michael's code. (Be sure to initialize it to a non-owned state.)

Mutex is a kernel-level synchronization primitive - it is more expensive than Monitor (that is what lock uses.).

Answer 4

Pass false for initiallyOwned in the mutex ctor. If you create the mutex and initially own it, you need to call ReleaseMutex again.

Answer 5

I have always avoided locking in a web-based application - let the web server deal with the threads, and instead build in duplicate detection.

What do you think you're going to get by locking on the CreateOrder? It seems to me that you may avoid creating two order simultaneously, but you're still going to end up with two orders created.