CODEWITHSUNDEEP
c#.netmultithreadingthread-safety
Luka Kovac
Luka Kovac

Reputation: 55

Is volatile a good practice to avoid race conditions in C#?

I was wondering if in the specific case like this one, where we have bool flag that indicates if some service is available:

    private bool isAvailable;
    private void foo()
    {
        if(isAvailable)
        {
            isAvailable = false;
            DoSomething();
            isAvailable = true;
        }
    }

Is it sufficient to set bool field to be volatile in a multithreaded environment or is it better to use locks or even Monitor.

In this specific case, if the service is not available at the moment, it is not needed to wait for it to get available again.

Upvotes: 1

Views: 1285

Answers (3)

Adem Catamak
Adem Catamak

Reputation: 2009

No it is not sufficient. Volatile is not enough for managing multithread, actually, volatile probably increase gap between expected result and actual result. Accessing variable (isAvailable) and modified it still leads to race condition. You may use lock, semaphore or atomic operation for managing multithread operation.

private object myLock = new object;
private void foo()
{
    lock(myLock)
    {
        DoSomething();
    }
}

or

[MethodImpl(MethodImplOptions.Synchronized)]
private void foo()
{
    // DoSomething
}

@Fildor is right. I miss consept of question, hence, I edit my answer. Code which is below, if resource is not locked, code lock resource and do something, else it skip critical section.

private object myLock = new object;
private void foo()
{
    if (Monitor.TryEnter(lockObject))
    {
        try
        {
            //do something
        }
        finally
        {
            Monitor.Exit(lockObject);
        }
    }
}

Upvotes: 6

Solomon Slow
Solomon Slow

Reputation: 27115

Other answers provide solutions to your problem, but just in case you were wondering why your solution won't work...

...Here's what could happen:

Thread A                                Thread B
--------                                --------
fetches isAvailable from memory
Tests the fetched value, it's true!     fetches isAvailable from memory
sets isAvailable=false                  Tests the fetched value, it's true!
calls DoSomething()                     sets isAvailable=false
...                                     calls DoSomething()
                                        ...

Upvotes: 1

ScottishTapWater
ScottishTapWater

Reputation: 4806

Adem Catamak has it absolutely right, locks are generally the way to go.

However, if your code is extremely performance critical you can have a look at using MemoryBarrier and volatile.

This page is extremely helpful in that regard: http://www.albahari.com/threading/part4.aspx

To add something to the lock answer, you can always abstract this away (not tested but you get the gist:

public class ThreadSafeObject
{
    private object _myObject; //Must be reference type
    public object MyObject
    {
        get
        {
            lock (_myObject)
            {
                return _myObject;
            }

        }
        set
        {
            lock (_myObject)
            {
                _myObject = value;
            }
        }
    }
}

You could even use generics to achieve this:

public class ThreadSafeGeneric<T>
{
    private T _myObject;
    public T MyObject
    {
        get
        {
            lock (_myObject)
            {
                return _myObject;
            }

        }
        set
        {
            lock (_myObject)
            {
                _myObject = value;
            }
        }
    }
}

Upvotes: 1

Related Questions