Usage of Mutex in c#

Question

I am a bit new in threading in c# and on general, in my program I am using mutex to allow only 1 thread getting inside a critical section and for unknown reason with doing some cw prints I can see that more than 1 thread is getting inside my critical section and this is my code :

Mutex m = new Mutex();
m.WaitOne();
<C.S> // critical section here
m.ReleaseMutex();

I would very much like to know if I am doing a mistake here thanks in advance for your kind help.

EDIT:

My code include classes so it basically looks more like this:

public class test
{
    private mutex m;
    public test()
    {
         m = new mutex();
    }
    public func()
    {
         m.WaitOne();
         <C.S> // critical section here
         m.ReleaseMutex();
     }


    } 

Answer 1

This pattern does no locking at all. Every thread creates a new Mutex object and immediately owns the lock for it. Other threads create and use a new Mutex itself.

Consider using a regular lock()!

lock(_lockobject) {
   // do inside what needs to be done - executed on a single thread only
} 

where _lockobject is a simple private variable in your class:

private object _lockobject; 

Edit: thanks to the commenters! Situations exist, where lock(this) can be dangerous. So I removed that.

Answer 2

It looks like you give each Thread its own Mutex. That won't work.

And a Mutex is overkill in most situations. You only need:

private static object syncLock = new object();  // just 1 instance

....

lock(syncLock)
{
    // critical section
}

Answer 3

Mutex use to identify run app instance.

 using (Mutex mutex = new Mutex(true, "app name", out createdNew))
            {
                if (createdNew)//check app is already run
                {
                    KillOthers();
                    StartApp();
                }
                else
                {
                    MessageBox.Show("Another instance already running!");
                }
            }

Answer 4

The problem here is that all your callers are using a different mutex; you need the locking object to be shared, usually by making it a field. For example, and switching to a simpler lock metaphor:

private readonly object syncLock = new object();
public void ThreadSafeMethod() {
    lock(syncLock) {
        /* critical code */
    }
}

or using the mutex:

private readonly Mutex m = new Mutex();
public void ThreadSafeMethod() {
    m.WaitOne();
    try {
        /* critical code */
    } finally {
        m.ReleaseMutex();
    }
}