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.

_"A mutex has thread-affinity, the owner of a mutex is a thread. The thread that acquired it must also be the one that calls ReleaseMutex(). Which is why your code bombs."_ - https://stackoverflow.com/a/9017728/43846 — stuartd, Jul 28 '20 at 12:31
This error means you are trying to release mutex from another thread, not one which took ownership. Without [mcve] it's hard to add more. — Sinatr, Jul 28 '20 at 12:31
Does this answer your question? [Releasing a Mutex](https://stackoverflow.com/questions/17070583/releasing-a-mutex) — Sinatr, Jul 28 '20 at 12:35
I believe the problem is race condition between `TryOpenExisting` which return `false` and attempting to create mutex with ownership in `else`. You may not succeed. You are not handling this case as per answer in duplicate. — Sinatr, Jul 28 '20 at 12:36
[Does this answer your question?](https://stackoverflow.com/questions/18690537/multi-processes-readwrite-one-file) — aepot, Jul 28 '20 at 12:37

score 2 · Accepted Answer · answered Jul 28 '20 at 12:54

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.

Thank you. I didn't know that the program may not wait for other threads on new Mutex(true, name); Also I don't know what race conditions exactly means. Could you explain or send a link? — Dadkhah Dadkhah, Jul 28 '20 at 19:17
@DadkhahDadkhah There's [an article about race conditions on Wikipedia](https://en.wikipedia.org/wiki/Race_condition), but basically it means you have some code which can go wrong when two threads call it almost simultaneously. — Matthew Watson, Jul 29 '20 at 07:56

Why my code doesn't work correctly in synchronization?

1 Answers1