Mutual exclusion: is this safe?

Question

Is this pattern for mutual exclusion as safe as I think it is? If so, what do you call it?

lock (_lock) {
    if (_flag) return;
    else _flag = true;
}
try {
    //critical code...
}
finally {
    _flag = false;
}

I want to ensure the critical section, but without the other threads piling up waiting to acquire the lock. Obviously I make sure the flag is set nowhere else. Is there a better way?

Answer 1

No, that is not safe. If you want to ensure mutually exclusive without blocking, you can use Monitor.TryEnter:

if (Monitor.TryEnter(lockObj, 0)) {
    // got the lock !
    try {
        // code
    }
    finally { // release the lock
        Monitor.Exit(lockObj);
    }
}

Answer 2

Have you looked at Monitor.TryEnter?

Answer 3

The correctness of your mutual exclusion pattern depends on the assignment _flag=false being atomic. Imagine what would happen if the assignment could be interrupted by another thread. If intermediate results of the assignment could be interpreted as false by the test, one assignment could cause several threads to enter the critical section.

The correctness of the mutual exclusion pattern also depends on the absence of optimisations in the compiler that may rearrange the ordering of the statements. Imagine a "smart" compiler that would move the assignment _flag=false up, because _flag is not referred to in the code that is in between (and the code in between does not throw exceptions). The compiler could then optimise the part in the lock section to read

if(_flag) return;

Both examples of why the pattern could fail are highly speculative and I think that you are safe in assuming it works. However, in the presence of another option that works as required, you're better off using that (see the other posts). If there are other developers in the same code, they do not need to consider whether the pattern works.

Answer 4

Wouldn't a simple lock(Object) statement work? Behind the scenes, it creates the Monitor and critical section inside a try... finally block.

private static readonly Object lockMe = new Object();
lock(lockMe)
{
    // critical code
}