CODEWITHSUNDEEP
c#.netmultithreadingvolatile
Jon
Jon

Reputation: 40032

Using volatile keyword and a lock statement

I am getting the "a reference to a volatile field will not be treated as volatile" warning in an application. I understand why.

As a simple example will the below code make the issue thread safe even though I will get the warning still?

private volatile int myVal = 10;
private int myNonVolatileNumber = 50;
private static readonly object lockObject = new object();

private void ChangeValue(ref int Value)
{
  lock (lockObject)
  {
    Value = 0;
  }
}

private void MyMethod()
{
  ChangeValue(ref myVal); //Warning here
  ChangeValue(ref myNonVolatileNumber); //no warning
}

Upvotes: 1

Views: 1709

Answers (4)

Jodrell
Jodrell

Reputation: 35716

What would be wrong with

private int val = 10;
private var valLock = new object();
private int nonVolatileNumber = 50;
private var nonVolatileNumberLock = new object();

public int Value
{
    get { lock(valLock) return val; }
    set { lock(valLock) val = value; }
}

public int NonVolatileNumber
{
    get { lock(nonVolatileNumberLock) return nonVolatileNumber; }
    set { lock(nonVolatileNumberLock) nonVolatileNumber = value; }
}

, the only risk here is that subsequent code accesses the property's private member.

In the case of 32bit integers, or even 64bit integers on a 64bit system, becasue the reads wil be atomic, you could use the Interlocked class like this ...

private int val = 10;

public int Value
{
    get { return val; }
    set { Interlocked.Exchange(ref val, value); }
}

Or in the case of a more complex type you could use ReadWriterLockSlim ...

private SomeStructure complex;
private var complexLock = new ReadWriterLockSlim();

public SomeStructure Complex
{
    get
    {
        complexLock.EnterReadLock();
        try
        {
            return complex;
        }
        finally
        {
            complexLock.ExitReadlock();
        }
    }
    set
    {
        complexLock.EnterWriteLock();
        try
        {
            return complex;
        }
        finally
        {
            complexLock.ExitWritelock();
        }
    }
}

This is better than a standard lock because it allows multiple simultaneous reads.

Upvotes: 0

S2S2
S2S2

Reputation: 8502

Probably there is no need for the usage of the volatile keyword at the place you have used.

This SO Question answers where all should the volatile keyword should be used if at all:

When should the volatile keyword be used in C#?

Upvotes: 1

dasblinkenlight
dasblinkenlight

Reputation: 151

Locking forces memory barriers on both sides, so yes, your example is thread safe.

Upvotes: 2

Henk Holterman
Henk Holterman

Reputation: 273244

You almost answer it yourself:

ChangeValue(ref myVal); //Warning here
ChangeValue(ref myNonVolatileNumber); //no warning

There is only 1 copy of the compiled ChangeValue(), the code inside should implement the 'volatile' behaviour. But the compiler (Jitter) cannot predict all calls when it compiles. The only option would be to treat every ref parameter as volatile, that would be very inefficient.

But see @Steven's comment, volatile is as good as useless and should be avoided.

Upvotes: 1

Related Questions