How to prevent a method from running across multiple threads?

Question

I am working on a web application, where several users can update the same record. So to avoid a problem if users are updating the same record at the same time, I am saving their changes in a queue. When each save occurs, I want to call a method that processes the queue on another thread, but I need to make sure that the method cannot run in another thread if it is called again. I’ve read several posts on the subject, but not sure what is best for my situation. Below is the code I have now. Is this the correct way to handle it?

public static class Queue {
    static volatile bool isProcessing;
    static volatile object locker = new Object();

    public static void Process() {
        lock (locker) {
            if (!isProcessing) {
                isProcessing = true;

                //Process Queue...

                isProcessing = false;
            }
        }
    }
}

Answer 1

New answer

If you are persisting these records to a database (or data files, or similar persistence system) you should let that underlying system handle the synchronization. As JohnSaunders pointed out Databases already handle simultaneous updates.

---

Given you want to persist the records… the problem presented by John is that you are only synchronizing the access to the data in a single instance of the web application. Still, there could be multiple instances running at the same time (for example in a server farm, which may be a good idea if you have high traffic). In this scenario using a queue to prevent simultaneous writes is not good enough because there is still a race condition among the multiple instances of the web page.

In that case, when you get updates for the same record from different instances, then the underlying system will have to handle the collision anyway, yet it will not be able to do it reliably because the order of the updates has been lost.

In addition to that problem, if you are using this data structure as a cache, then it will provide incorrect data because it is not aware of the updates that happen in another instance.

---

With that said, for the scenarios where it may be worth to use a Thread-Safe Queue. For those cases you could use ConcurrentQueue (as I mention at the end of my original answer).

I'll keep my original answer, because I see value in helping understand the threading synchronization mechanism available in .NET (of which I present a few).

---

Original answer

Using lock is enough to prevent the access of multiple threads to a code segment at the same time (this is mutual exclusion).

Here I have commented out what you don't need:

public static class Queue {
    // static volatile bool isProcessing;
    static /*volatile*/ object locker = new Object();

    public static void Process() {
        lock (locker) {
            // if (!isProcessing) {
            //  isProcessing = true;

                //Process Queue...

            //  isProcessing = false;
            // }
        }
    }
}

The lock does NOT need volatile to work. However you might still need the variable to be volatile due to other code not included here.

---

With that said, all the threads that try to enter in the lock will be waiting in a queue. Which as I understand is not what you want. Instead you want all the other threads to skip the block and leave only one do the work. This can be done with Monitor.TryEnter:

public static class Queue
{
    static object locker = new Object();

    public static void Process()
    {
        bool lockWasTaken = false;
        try
        {
            if (Monitor.TryEnter(locker))
            {
                lockWasTaken = true;
                //Process Queue…
            }
        }
        finally
        {
            if (lockWasTaken)
            {
                Monitor.Exit(locker);
            }
        }
    }
}

---

Another good alternative is to use Interlocked:

public static class Queue
{
    static int status = 0;

    public static void Process()
    {
        bool lockWasTaken = false;
        try
        {
            lockWasTaken = Interlocked.CompareExchange(ref status, 1, 0) == 0;
            if (lockWasTaken)
            {
                //Process Queue…
            }
        }
        finally
        {
            if (lockWasTaken)
            {
                Volatile.Write(ref status, 0);
                // For .NET Framework under .NET 4.5 use Thread.VolatileWrite instead.
            }
        }
    }
}

---

Anyway, you don't have the need to implement your own thread-safe queue. You could use ConcurrentQueue.

Answer 2

A lock is good but it won't work for async await. You will get the following error if you try to await a method call in a lock:

CS1996 Cannot await in the body of a lock statement

In this case you should use a SemaphoreSlim

Example:

public class TestModel : PageModel
{
    private readonly ILogger<TestModel> _logger;
    private static readonly SemaphoreSlim _semaphoreSlim = new SemaphoreSlim(1, 1);

    public TestModel(ILogger<TestModel> logger)
    {
        _logger = logger;
    }

    public async Task OnGet()
    {
        await _semaphoreSlim.WaitAsync();
        try
        {
            await Stuff();
        }
        finally
        {
            _semaphoreSlim.Release();
        }
    }
}

It is important to not new SemaphoreSlim in the constructor or anywhere else because then it won't work.

https://stackoverflow.com/a/18257065/3850405

https://docs.microsoft.com/en-us/dotnet/api/system.threading.semaphoreslim?view=net-5.0