4

This is a simple problem, but after reading Why do I need a memory barrier? I'm very confused about it.

In the example below, assume different threads are repeatedly calling Increment and Counter:

class Foo{
    int _counter=0;
    public int Counter 
    {
        get { return _counter; }
    }

    public void Increment()
    {
        Interlocked.Increment(ref _counter);
    }
}

Sorry if I'm misinterpreting Why do I need a memory barrier? but it seems like it's suggesting the class above might not be providing a freshness guarantee when reading the value of _counter. Could a thread that's repeatedly accessing the Counter property get forever stuck on an old value of Counter (because it is cached in the register)?

Is a memory barrier or a lock before return _counter; necessary?

Community
  • 1
  • 1
marinosb
  • 5,367
  • 2
  • 18
  • 10

1 Answers1

8

Is a memory barrier or a lock before return _counter; necessary?

Yes, absolutely. Consider the following code.

while (foo.Counter == 0)
{
  // Do something
}

The problem is that if the contents inside the loop are simple enough then the C# compiler, JIT compiler, or hardware will optimize the code in this manner.

int register = foo._counter;
while (register == 0)
{
  // Do something
}

Or even this.

if (foo._counter == 0)
{
  START: 
  // Do something
  goto START;
}

Notice that I use _counter instead of Counter as a way of implying that the property will probably be inlined. Then, more importantly, the JIT compiler may "lift" the read of _counter outside of the loop so that it is only read once.

Memory barriers do not provide a freshness guarantee per se. What they do is prevent certain software or hardware optimizations that reorder reads and writes to memory. The freshness guarantee is more of a side effect really.

So to wrap things up your Counter property should look like this.

public int Counter 
{
    get { return Interlocked.CompareExchange(ref _counter, 0, 0); }
}
Brian Gideon
  • 47,849
  • 13
  • 107
  • 150
  • Thanks Brian! I couldn't have expected a clearer answer. Instead of wrapping the read in `Interlocked`, could we achieve the same effect by using `Thread.MemoryBarrier()` before `return _counter;`? I'm just curious as to what the most optimal solution would be – marinosb Mar 24 '12 at 01:44
  • @sifester: Oh you're really going to enjoy [this answer](http://stackoverflow.com/a/8417285/158779) at the end where I talk about that very thing. In short, yes you can use `Thread.MemoryBarrier`, but the placement should be *after* the read. I know...it's enough to drive any programmer mad! – Brian Gideon Mar 24 '12 at 03:41
  • Is there any reason why you're using `Interlocked.CompareExchange()` instead of `Thread.VolatileRead()`? – svick Mar 24 '12 at 13:44
  • @svick: Not really no. Except that `Interlocked` was already used elsewhere so it seemed logical to stick with that paradigm if nothing else than for consistency. `Thread.VolatileRead` works just fine though. – Brian Gideon Mar 24 '12 at 14:21