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

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?

Jon Skeet
people
quotationmark

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.

people

See more on this question at Stackoverflow