Why my code doesn't work correctly in synchronization?

Question

I'm using this class to prevent two application (one windows application and one web application running on the same OS) using a shared file at the same time. But I get the error "Object synchronization method was called from an unsynchronized block of code."

    class SharedMutex : IDisposable
    {
        readonly Mutex mutex;

        /// <summary>
        /// This function will wait if other thread has owned the mutex
        /// </summary>
        /// <param name="name"></param>
        public SharedMutex(string name)
        {
            bool m = Mutex.TryOpenExisting(name, out mutex);
            if (m)
            {
                mutex.WaitOne();
            }
            else
            {
                mutex = new Mutex(true, name);
            }
        }

        public const string Logs = @"Global\Logs";

        public void Dispose()
        {
            mutex.ReleaseMutex();
            mutex.Dispose();
        }
    }

And this is the way I'm using this class

using (new SharedMutex(SharedMutex.Logs))
                {
                   ///access the shared file
                }

This class exists in both projects.

Note: I am not looking for a solution to the problem to access the files, I need to know why my code has problem. Because I want to use this code for other purposes also. Thank you.

Answer 1

I think that this is likely caused by a race condition (as suggested by /u/Sinatr in the comments to the question).

The following code reproduces the issue:

using System;
using System.Threading;
using System.Threading.Tasks;

namespace ConsoleApp1
{
    class Program
    {
        static void Main()
        {
            var t1 = Task.Run(func1);
            var t2 = Task.Run(func2);

            Task.WaitAll(t1, t2);

            Console.WriteLine("Done. Press <ENTER>");
            Console.ReadLine();
        }

        static void func1()
        {
            using (new SharedMutex("test"))
            {
                Thread.Sleep(2000);
            }
        }

        static void func2()
        {
            using (new SharedMutex("test"))
            {
                Thread.Sleep(1000);
            }
        }
    }

    class SharedMutex : IDisposable
    {
        readonly Mutex mutex;

        public SharedMutex(string name)
        {
            bool m = Mutex.TryOpenExisting(name, out mutex);

            if (m)
            {
                mutex.WaitOne();
            }
            else
            {
                Thread.Sleep(10); // Simulate a short delay.
                mutex = new Mutex(true, name);
            }
        }

        public void Dispose()
        {
            mutex.ReleaseMutex();
            mutex.Dispose();
        }
    }
}

Note the short delay Thread.Sleep(10) which is enough to provoke the exception on my system, running the debug build. The delay may have to be increased to provoke the exception on other systems.

If this is indeed the problem, this is how to fix it:

class SharedMutex : IDisposable
{
    readonly Mutex mutex;

    public SharedMutex(string name)
    {
        mutex = new Mutex(false, name);
        mutex.WaitOne();
    }

    public void Dispose()
    {
        mutex.ReleaseMutex();
        mutex.Dispose();
    }
}

You don't really care if it was newly created. The Mutex constructor takes care of that for you.

Even if this isn't the problem, I still think you should use the implementation above, since it does not have a potential race condition.

As the documentation for the constructor Mutex(bool, string) states:

If name is not null and initiallyOwned is true, the calling thread owns the mutex only if the named system mutex was created as a result of this call. Since there is no mechanism for determining whether the named system mutex was created, it is better to specify false for initiallyOwned when calling this constructor overload.