Mutex/Monitor not working for C# Thread synchronization

Question

I am trying to only allow one Thread to access a non-static method from an object. I don't really create Threads but use Tasks instead. I tried to lock the method down to one usage but it just isn't working, I first tried it with a static Mutex object, then a non-static Mutex object but even with a simple monitor it's not working.

My code:

private async void LoadLinkPreview()
    {
        try
        {
            // TODO: FEHLERTRÄCHTIG!
            Monitor.Enter(_loadLinkMonitor);
            Debug.WriteLine(DateTime.Now + ": Entered LinkPreview");

            // Code ...
            // There are also "await" usages here, don't know if this matters
        }
        catch { }
        finally
        {
             Monitor.Exit(_loadLinkMonitor);
             Debug.WriteLine(DateTime.Now + ": Left LinkPreview");
        }
    }

and this is what the Console displays:

3/12/2013 6:10:03 PM: Entered LinkPreview
3/12/2013 6:10:05 PM: Entered LinkPreview
3/12/2013 6:10:05 PM: Left LinkPreview
3/12/2013 6:10:06 PM: Left LinkPreview

Of course sometimes it works in the wanted order but I want it to always work. Can someone help me maybe?

EDIT: I realized that the console display could be wanted behavior, since the "Left" WriteLine() is after the monitor has been exited, but I switched them now and can confirm that there are definitely two threads working at the same time! Also a more obvious console result:

3/12/2013 6:26:15 PM: Entered LinkPreview
3/12/2013 6:26:15 PM: Entered LinkPreview
2 had an error! // Every time the method is used I count up now, this confirms that the second
// time the method is called the error is produced, which would not happen if the monitor
// is working because I have an if statement right upfront ...
3/12/2013 6:26:17 PM: Left LinkPreview
3/12/2013 6:26:18 PM: Left LinkPreview

That inner await probably matters. Not sure if it applies to a monitor, but if a thread tried to lock() something twice (nested), it is allowed to do so without being blocked. You would get the behavior you noted. — Meirion Hughes, Mar 12 '13 at 17:22

score 4 · Accepted Answer · answered Mar 12 '13 at 17:27

4

It's actually possible that your code is working as intended, and your problem is just with the display.

The issue is here:

 Monitor.Exit(_loadLinkMonitor);
 Debug.WriteLine(DateTime.Now + ": Left LinkPreview");

You write out the timestamp after you exit the monitor. Keep in mind that the other code that is waiting to enter the monitor can start executing as soon as Exit is called. So it's possible for the current thread to call exit, then before the Debug call, the other thread can Enter the monitor and write it's on "Entered" line.

If you change the code to:

 Debug.WriteLine(DateTime.Now + ": Left LinkPreview");
 Monitor.Exit(_loadLinkMonitor);

Then it won't change the behavior, but you'll remove the logging bug. If it's still out of order, then you do indeed have a problem somewhere in your code.

On a side note, if you have an async method you should avoid calling methods that do a blocking wait, such as Monitor.Enter. If you use SemaphoreSlim you can use it's WaitAsync method to ensure that the method doesn't block. That would look something like this:

private SemaphoreSlim semaphore = new SemaphoreSlim(1);
private async void LoadLinkPreview()
{
    try
    {
        await semaphore.WaitAsync();
        Debug.WriteLine(DateTime.Now + ": Entered LinkPreview");
        await Task.Delay(2000);//placeholder for real work/IO
    }
    finally
    {
        Debug.WriteLine(DateTime.Now + ": Left LinkPreview");
        semaphore.Release();
    }
}

answered Mar 12 '13 at 17:27

Servy

202,030
26
332
449

Hi thanks, I edited my answer already I saw that there was something unclear =) I will try the SemaphoreSlim, it sounds great, but it should work anyways or not? – Smirnoff4u Mar 12 '13 at 17:31
@Smirnoff4u The second half, referring to using a `SemaphoreSlim` is not related to your problem, it's just an improvement of your code. The problem is what I discuss at the start; the ordering of your logging vs your locking operations. – Servy Mar 12 '13 at 17:32
Yes, but I switched them now and it still doesn't work, so that wasn't the issue. – Smirnoff4u Mar 12 '13 at 17:33
@Smirnoff4u Then it's probably related to code that you haven't shown. – Servy Mar 12 '13 at 17:33
SemaphoreSlim works beautifully! Thank you very much, I think this also explains issues I had in other applications! A follow up question: If I need a mutex for process synchronization, would something like this be fine? public async static void F() { await _semaSlim.WaitAsync(); await DoMutexWork(); _semaSlim.Release(); } private static void MutexWork() { Mutex.WaitOne() // Storage stuff Mutex.ReleaseOne() } – Smirnoff4u Mar 12 '13 at 17:46
@Smirnoff4u I would think so. – Servy Mar 12 '13 at 17:50

score 4 · Answer 2 · edited May 23 '17 at 11:44

4

Edit: ditched all of that. Here's the answer: Why can't I use the 'await' operator within the body of a lock statement?

You aren't supposed to do what you're trying to do, using "await" inside of essentially a "lock" statement.

edited May 23 '17 at 11:44

Community

1
1

answered Mar 12 '13 at 17:37

Kevin Anderson

6,850
4
32
54

score 0 · Answer 3 · answered Mar 12 '13 at 18:13

You can't use standard multithreading primitives such as Monitor in an async method. The C# compiler will stop you if you use a lock statement, but if you do it "manually" via Monitor.Enter then you will just end up shooting yourself in the foot.

If you think about it for a bit, it makes perfect sense. What happens when an async method yields? It returns an uncompleted Task and allows other code to run while it's (a)waiting. That totally goes against the point of a lock, which is to prevent other code from running while the lock is held. There are also situations (such as an ASP.NET or thread pool context) where the async method resumes on a different thread; in this case, one thread takes a lock and another thread releases it. Insanity! Cats and dogs living together!

The built-in .NET solution is SemaphoreSlim, which works as a lock/semaphore (though it does not have built-in IDisposable for use with using). Stephen Toub wrote a blog series on async coordination primitives, and I have a full suite implemented in my AsyncEx library, available via NuGet.

Mutex/Monitor not working for C# Thread synchronization

3 Answers3