Reading an int that's updated by Interlocked on other threads

Question

(This is a repeat of: How to correctly read an Interlocked.Increment'ed int field? but, after reading the answers and comments, I'm still not sure of the right answer.)

There's some code that I don't own and can't change to use locks that increments an int counter (numberOfUpdates) in several different threads. All calls use:

Interlocked.Increment(ref numberOfUpdates); 

I want to read numberOfUpdates in my code. Now since this is an int, I know that it can't tear. But what's the best way to ensure that I get the latest value possible? It seems like my options are:

int localNumberOfUpdates = Interlocked.CompareExchange(ref numberOfUpdates, 0, 0); 

Or

int localNumberOfUpdates = Thread.VolatileRead(numberOfUpdates); 

Will both work (in the sense of delivering the latest value possible regardless of optimizations, re-orderings, caching, etc.)? Is one preferred over the other? Is there a third option that's better?

Answer 1

I'm a firm believer in that if you're using interlocked to increment shared data, then you should use interlocked everywhere you access that shared data. Likewise, if you use insert you favorite synchronization primitive here to increment shared data, then you should use insert you favorite synchronization primitive here everywhere you access that shared data.

int localNumberOfUpdates = Interlocked.CompareExchange(ref numberOfUpdates, 0, 0); 

Will give you exactly what your looking for. As others have said interlocked operations are atomic. So Interlocked.CompareExchange will always return the most recent value. I use this all the time for accessing simple shared data like counters.

I'm not as familiar with Thread.VolatileRead, but I suspect it will also return the most recent value. I'd stick with interlocked methods, if only for the sake of being consistent.

---

Additional info:

I'd recommend taking a look at Jon Skeet's answer for why you may want to shy away from Thread.VolatileRead(): Thread.VolatileRead Implementation

Eric Lippert discusses volatility and the guarantees made by the C# memory model in his blog at http://blogs.msdn.com/b/ericlippert/archive/2011/06/16/atomicity-volatility-and-immutability-are-different-part-three.aspx. Straight from the horses mouth: "I don't attempt to write any low-lock code except for the most trivial usages of Interlocked operations. I leave the usage of "volatile" to real experts."

And I agree with Hans's point that the value will always be stale at least by a few ns, but if you have a use case where that is unacceptable, its probably not well suited for a garbage collected language like C# or a non-real-time OS. Joe Duffy has a good article on the timeliness of interlocked methods here: http://joeduffyblog.com/2008/06/13/volatile-reads-and-writes-and-timeliness/

Answer 2

Thread.VolatileRead(numberOfUpdates) is what you want. numberOfUpdates is an Int32, so you already have atomicity by default, and Thread.VolatileRead will ensure volatility is dealt with.

If numberOfUpdates is defined as volatile int numberOfUpdates; you don't have to do this, as all reads of it will already be volatile reads.

---

There seems to be confusion about whether Interlocked.CompareExchange is more appropriate. Consider the following two excerpts from the documentation.

From the Thread.VolatileRead documentation:

Reads the value of a field. The value is the latest written by any processor in a computer, regardless of the number of processors or the state of processor cache.

From the Interlocked.CompareExchange documentation:

Compares two 32-bit signed integers for equality and, if they are equal, replaces one of the values.

In terms of the stated behavior of these methods, Thread.VolatileRead is clearly more appropriate. You do not want to compare numberOfUpdates to another value, and you do not want to replace its value. You want to read its value.

---

Lasse makes a good point in his comment: you might be better off using simple locking. When the other code wants to update numberOfUpdates it does something like the following.

lock (state) {     state.numberOfUpdates++; } 

When you want to read it, you do something like the following.

int value; lock (state) {     value = state.numberOfUpdates; } 

This will ensure your requirements of atomicity and volatility without delving into more-obscure, relatively low-level multithreading primitives.