Lock Using “Interlocked” vs lock Statement (“Monitor”) (Followup)
void Unlock()
{
    Interlocked.CompareExchange(ref _locked, Unlocked, Locked);
}


So, if you try to unlock when the lock is not locked, nothing happens? That sounds like a recipe for well-hidden bugs. Instead, unlocking when not locked should throw an exception.



_millisecondsTimeout = millisecondsTimeout &lt; -1 ? -1 : millisecondsTimeout;


Why are you making sure that negative values are changed to -1, when later on you just check whether the timeout is negative?

Anyway, the approach taken by .Net methods like SpinUntil() is that negative values other than -1 throw, so I think you should do that too to be consistent.



if (_millisecondsTimeout &lt; 0) _spinMutex.Lock();
else if (!_spinMutex.Lock(_millisecondsTimeout)) throw new TimeoutException();
else _locked = true;


I don't like having the three branches on the same level here, because logically, they're not. I think this way makes more sense:

if (_millisecondsTimeout &lt; 0)
{
    _spinMutex.Lock();
}
else
{
    if (!_spinMutex.Lock(_millisecondsTimeout)) throw new TimeoutException();
    else _locked = true;
}


Also, what's the purpose of the first if at all. Removing it will work the same:

if (!_spinMutex.Lock(_millisecondsTimeout)) throw new TimeoutException();
else _locked = true;




catch
{
    Dispose();
    _shouldDispose = false;
    throw;
}


Calling Dispose() here makes sense only if the lock gets locked (so TimeoutException wasn't thrown) but then an exception was thrown. But most of the time, such exceptions (e.g. ThreadAbortException) are catastrophic and the whole process is going to go down, so you don't need to care about them much.



_shouldDispose = false;


I don't understand this. If the constructor throws an exception, the caller never gets their hands on the created instance, so they won't be able to call Dispose(). This means that _shouldDispose = false here doesn't make much sense.

On the other hand, calling Dispose() more than once should work (“If an object's Dispose method is called more than once, the object must ignore all calls after the first one.”), but won't, so adding _shouldDispose = false directly into Dispose() would make sense.



I think that _locked should be set always when the lock is locked, not just under some circumstances. That would make understanding your code easier and it would also simplify code in both in the constructor and in Dispose().