Is Monitor.TryEnter(lockObject, timeout) overload unsafe? (.net)

Question

I was suggested during a code review to do

bool acquiredLock = false;
try {
    Monitor.TryEnter(lockObject, 500, ref acquiredLock);
    if (acquiredLock) {
        // do something
    }
    else {
        // fallback strategy
   }
}
finally
{
    if (acquiredLock)
    {
        Monitor.Exit(lockObject);
    }
}

instead of the simpler

if (Monitor.TryEnter(lockObject, 500)) {
      try {
          // do something...
      } 
      finally {
          Monitor.Exit(lockObject);
      }
} else {
     // fallback strategy
}

What difference does it make? How can the first code not exhibit a bug where the second would exhibit a bug?

@SLaks Sorry, that was due to a mistake in the question. That's not the actual question. I'll fix the question. — Tim Lovell-Smith, Feb 24 '15 at 18:41
You'll have to throw a Thread.Abort() in the mix and reason if your code still works properly. Backgrounder [is here](http://stackoverflow.com/a/14831590/17034), x64 jitter required to mess it up. — Hans Passant, Feb 24 '15 at 18:44
Probably a duplicate of http://stackoverflow.com/questions/3674650/what-important-difference-exists-between-monitor-tryenterobject-and-monitor-tr — xanatos, Feb 24 '15 at 18:49
Yes, that looks like a duplicate to me, even though all the function signatures are different! — Tim Lovell-Smith, Feb 24 '15 at 20:37

Jon Skeet · Accepted Answer · 2015-02-24T18:47:06.517

Assuming in your second snippet you'd actually call Monitor.Exit, the difference is explained in the documentation:

This overload always sets the value of the variable that is passed to the ref parameter lockTaken, even if the method throws an exception, so the value of the variable is a reliable way to test whether the lock has to be released.

In other words, with your second snippet, it might be feasible for an asynchronous exception to be thrown (e.g. the thread being aborted) after the lock has been acquired, but before the method has returned. Even with a finally block, you couldn't then easily tell whether or not you'd need to release the lock. Using the ref parameter, the "monitor acquired" and "ref parameter set to true" actions are atomic - it's impossible for the variable to have the wrong value when the method exits, however it exits.

As of C# 4, when targeting a platform supporting this overload, this is the code that the C# compiler generates too.

score 0 · Answer 2 · answered Feb 24 '15 at 18:38

0

Your first snippet is exiting the monitor when the second is not. You want to release the monitor when you're done with the critical block.

answered Feb 24 '15 at 18:38

Servy

202,030
26
332
449

Sorry, that was due to a mistake in the question. That's not the actual question. I'll fix the question. – Tim Lovell-Smith Feb 24 '15 at 18:40

score 0 · Answer 3 · answered Feb 24 '15 at 18:40

0

Besides the entirely valid point about exiting that other posters have made... The documentation for the TryEnter method states that it can throw one of three exceptions, so technically, yes it could bomb your application under specific circumstances.

answered Feb 24 '15 at 18:40

djdanlib

21,449
1
20
29

Those documented exceptions are all argument validation exceptions. It seems highly unlikely you would get any of them after acquiring the lock. – Tim Lovell-Smith Feb 24 '15 at 20:37
Indeed. However: The statement Monitor.TryEnter is not enclosed in something that would catch those validation exceptions, at least not in the code snippet presented. It's possible that it could bomb at runtime. – djdanlib Feb 27 '15 at 21:43
What I mean is if you get any of those exceptions, obviously you haven't got the lock. You don't even need the ref lockTaken parameter to know that. – Tim Lovell-Smith Feb 27 '15 at 21:48

score 0 · Answer 4 · answered Feb 24 '15 at 18:41

Even tough the method is named TryEnter, it actually throws exceptions, and you need to handle them properly. If it throws an exception and you don't handle it and release monitor, you can have a deadlock situation.

This maybe a bit too precautious, because if you look at exception that it actually throws, it will most probably throw them BEFORE trying to acquire lock. But this is an implementation detail and you can't be sure about it.

You can probably check how this method is implemented at http://referencesource.microsoft.com/ but, as I said, this is an implementation detail.

Is Monitor.TryEnter(lockObject, timeout) overload unsafe? (.net)

4 Answers4