views:

345

answers:

3

In my multithreaded asmx web service I had a class field _allData of my own type SystemData which consists of few List<>s and Dictionary<>s marked as volatile. The system data (_allData) is refreshed once in a while and I do it by creating another object called newData and fill it's data structures with new data. When it's done I just assign

private static volatile SystemData _allData

public static bool LoadAllSystemData()
{
    SystemData newData = new SystemData();
    /* fill newData with up-to-date data*/
     ...
    _allData = newData.
} 

This should work since the assignment is atomic and the threads that have the reference to old data keep using it and the rest have the new system data just after assignment. However my collegue said that instead of using volatile keyword and simple assigment I should use InterLocked.Exchage because he said that on some platforms it's not guaranteed that reference assignment is atomic. Moreover: when I declare the _allData field as volatile the

Interlocked.Exchange<SystemData>(ref _allData, newData); 

produces warning "a reference to a volatile field will not be treated as volatile" What should I think about this?

BR: Matti

+4  A: 

Either your collegue is mistaken, or he knows something that the C# language specification doesn't.

5.5 Atomicity of variable references:

"Reads and writes of the following data types are atomic: bool, char, byte, sbyte, short, ushort, uint, int, float, and reference types."

So, you can write to the volatile reference without risk of getting a corrupted value.

You should of course be careful with how you decide which thread should fetch the new data, to minimise the risk that more than one thread at a time does that.

Guffa
True as long as they are memory aligned correctly.
zebrabox
@guffa: yes I've read that also. this leaves the original question "reference assignment is atomic so why is Interlocked.Exchange(ref Object, Object) needed?" unaswered
matti
@zebrabox: what do you mean? when they are not? what would you do?
matti
@matti: It's needed when you have to read and write a value as an atomic operation.
Guffa
How often do you have to worry about the memory not being aligned correctly in .NET really? Interop-heavy stuff?
Skurmedel
+3  A: 

Interlocked.Exchange< T >

Sets a variable of the specified type T to a specified value and returns the original value, as an atomic operation.

It changes and returns the original value, it's useless because you only want to change it and, as Guffa said, it's already atomic.

Unless a profiler as proven it to be a bottleneck in your application, you should consider unsing locks, it's easier to understand and prove that your code is right.

Guillaume
+12  A: 

There are numerous questions here. Considering them one at a time:

reference assignment is atomic so why is Interlocked.Exchange(ref Object, Object) needed?

Reference assignment is atomic. Interlocked.Exchange does not do only reference assignment. It does a read of the current value of a variable, stashes away the old value, and assigns the new value to the variable, all as an atomic operation.

my colleague said that on some platforms it's not guaranteed that reference assignment is atomic. Was my colleague correct?

No. Reference assignment is guaranteed to be atomic on all .NET platforms.

My colleague is reasoning from false premises. Does that mean that their conclusions are incorrect?

Not necessarily. Your colleague could be giving you good advice for bad reasons. Perhaps there is some other reason why you ought to be using Interlocked.Exchange. Lock-free programming is insanely difficult and the moment you depart from well-established practices espoused by experts in the field, you are off in the weeds and risking the worst kind of race conditions. I am neither an expert in this field nor an expert on your code, so I cannot make a judgement one way or the other.

produces warning "a reference to a volatile field will not be treated as volatile" What should I think about this?

You should understand why this is a problem in general. That will lead to an understanding of why the warning is unimportant in this particular case.

The reason that the compiler gives this warning is because marking a field as volatile means "this field is going to be updated on multiple threads -- do not generate any code that caches values of this field, and make sure that any reads or writes of this field are not "moved forwards and backwards in time" via processor cache inconsistencies."

(I assume that you understand all that already. If you do not have a detailed understanding of the meaning of volatile and how it impacts processor cache semantics then you don't understand how it works and should not be using volatile. Lock-free programs are very difficult to get right; make sure that your program is right because you understand how it works, not right by accident.)

Now suppose you make a variable which is an alias of a volatile field by passing a ref to that field. Inside the called method, the compiler has no reason whatsoever to know that the reference needs to have volatile semantics! The compiler will cheerfully generate code for the method that fails to implement the rules for volatile fields, but the variable is a volatile field. That can completely wreck your lock-free logic; the assumption is always that a volatile field is always accessed with volatile semantics. It makes no sense to treat it as volatile sometimes and not other times; you have to always be consistent otherwise you cannot guarantee consistency on other accesses.

Therefore, the compiler warns when you do this, because it is probably going to completely mess up your carefully developed lock-free logic.

Of course, Interlocked.Exchange is written to expect a volatile field and do the right thing. The warning is therefore misleading. I regret this very much; what we should have done is implement some mechanism whereby an author of a method like Interlocked.Exchange could put an attribute on the method saying "this method which takes a ref enforces volatile semantics on the variable, so suppress the warning". Perhaps in a future version of the compiler we shall do so.

Eric Lippert
thanks! best answer ever!
matti