Monitor.TryEnter doesn't work

Question

Part of my code-behind:

object _sync = new object();

private async void OnKeyDown(object sender, KeyEventArgs e) {
    if (!Monitor.TryEnter(_sync)) return;

    Trace.Write("taken...");
    await Task.Delay(TimeSpan.FromSeconds(5));
    Trace.WriteLine(" done");

    Monitor.Exit(_sync);
}

Output (pressing several times in less than 5 seconds):

taken...taken...taken... done
done
done

How-come?? the _sync lock is never being taken, why?

Answer 1

Mixing Monitor and await is... more than a little risky. It looks like what you are trying to do is to ensure it only runs once at a time. I suspect Interlocked may be simpler:

object _sync = new object();
int running = 0;
private async void OnKeyDown(object sender, KeyEventArgs e) {
    if(Interlocked.CompareExchange(ref running, 1, 0) != 0) return;

    Trace.Write("taken...");
    await Task.Delay(TimeSpan.FromSeconds(5));
    Trace.WriteLine(" done");

    Interlocked.Exchange(ref running, 0);
}

Note you might also want to think what happens if an error occurs etc; how does the value become reset? You can probably use try/finally:

if(Interlocked.CompareExchange(ref running, 1, 0) != 0) return;

try {
    Trace.Write("taken...");
    await Task.Delay(TimeSpan.FromSeconds(5));
    Trace.WriteLine(" done");
} finally {
    Interlocked.Exchange(ref running, 0);
}

Answer 2

You can't use a thread-affine type like Monitor with await. In this particular case, you're always acquiring the lock on the same thread (the UI thread), and this type of lock permits recursive locking.

Try SemaphoreSlim instead (WaitAsync and Release instead of Enter and Exit):

SemaphoreSlim _sync = new SemaphoreSlim(1);

private async void OnKeyDown(object sender, KeyEventArgs e) {
  await _sync.WaitAsync();

  Trace.Write("taken...");
  await Task.Delay(TimeSpan.FromSeconds(5));
  Trace.WriteLine(" done");

  _sync.Release();
}

Answer 3

You can't use await between Monitor.TryEnter() and Monitor.Exit() method calls. After the await, the thread context might be different which would mean that the thread wouldn't have the entered the lock and therefore it would NOT be able to exit it.

In fact, the compiler will protect you if you use the lock keyword:

lock(_sync)
{
  await Task.Delay(...); // <- Compiler error...
}

Answer 4

TryEnter will run on your gui thread. It's valid for a thread to acquire a monitor multiple times without blocking, it just has to release them the same number of times.

Your call to Monitor.Exit will run in the the context dictated by yourasync call. If it ends up running on a thread other than the thread that called TryEnter then it will fail to release the monitor.

So, you're acquiring the monitor on the same thread each time, which will never block, and you're releasing it on some other thread, that may work. That's why you're able to click rapidly within the 5 second window.

Answer 5

What happens is that TryEnter will succeed if the current thread already acquired the lock. The KeyDown event will always fire on the Dispatcher thread, while a background thread is processing the wait and then enqueueing the unlock back on the dispatcher thread.

Answer 6

Stephen Cleary's answer using the SemaphoreSlim changes the behavior because it queues the clicks instead of discarding them. The solution is to add a timeout of 0ms to .Wait(0) or .WaitAsync(0):

https://docs.microsoft.com/en-us/dotnet/api/system.threading.semaphoreslim.wait?view=net-6.0#system-threading-semaphoreslim-wait(system-int32)

private readonly SemaphoreSlim _semaphoreSlim = new SemaphoreSlim(1);

private async void OnKeyDown(object sender, KeyEventArgs e)
{
    if (!_semaphoreSlim.Wait(0))
        return;

    try
    {
        Trace.Write("taken...");
        await Task.Delay(TimeSpan.FromSeconds(5));
        Trace.WriteLine(" done");
    }
    finally
    {
        _semaphoreSlim.Release();
    }
}