CODEWITHSUNDEEP
c#.netlockingsynchronized
Tommaso Belluzzo
Tommaso Belluzzo

Reputation: 23685

Best Practices Concerning Method Locking

I have a method whom access myst be synchronized allowing only one thread at once to go though it. Here is my current implementation:

private Boolean m_NoNeedToProceed;
private Object m_SynchronizationObject = new Object();

public void MyMethod()
{
    lock (m_SynchronizationObject)
    {
        if (m_NoNeedToProceed)
            return;

Now I was thinking about changing it a little bit like so:

private Boolean m_NoNeedToProceed;
private Object m_SynchronizationObject = new Object();

public void MyMethod()
{
    if (m_NoNeedToProceed)
        return;

    lock (m_SynchronizationObject)
    {

Shouldn't it be better to do a quick return before locking it so that calling threads can proceed without waiting for the previous one to complete the method call?

Upvotes: 3

Views: 973

Answers (3)

Branko Dimitrijevic
Branko Dimitrijevic

Reputation: 52117

Shouldn't it be better to do a quick return before locking it...

No. A lock is is not just a mutual-exclusion mechanism, it's also a memory barrier1. Without a lock, you could introduce a data race if any of the concurrent threads tries to modify the variable2.

BTW, locks have a good performance when there is no contention, so you wouldn't be gaining much performance anyway. As always, refrain from making assumptions about performance, especially this "close to the metal". If in doubt, measure!

...so that calling threads can proceed without waiting for the previous one to complete the method call?

This just means you are holding the lock for longer than necessary. Release the lock as soon as the shared memory no longer needs protection (which might be sooner than the method exit), and you won't need to try to artificially circumvent it.

1 I.e. triggers a cache coherency mechanism so all CPU cores see the "same" memory.

2 For example, one thread writes to the variable, but that change lingers in one core's write buffer for some time, so other threads on other cores don't see it immediately.

Upvotes: 1

Yochai Timmer
Yochai Timmer

Reputation: 49251

Yes it's better to that before you lock.

Make m_NoNeedToProceed volatile

Just a disclaimer: volatile doesn't make it thread safe. It just causes a barrier to check if the value has changed in another processor.

Upvotes: 0

Cemafor
Cemafor

Reputation: 1653

Yes, as long as m_NoNeedToProceed doesn't have any race conditions associated with it.

If the method takes a long time to run, and some threads do not need to actually access the critical sections of the method. Then it would be best to let them return early without getting the lock.

Upvotes: 0

Related Questions