views:

156

answers:

2

I've been reading the answer to a similar question, but I'm still a little confused... Abel had a great answer, but this is the part that I'm unsure about:

...declaring a variable volatile makes it volatile for every single access. It is impossible to force this behavior any other way, hence volatile cannot be replaced with Interlocked. This is needed in scenarios where other libraries, interfaces or hardware can access your variable and update it anytime, or need the most recent version.

Does Interlocked guarantee visibility of the atomic operation to all threads, or do I still have to use the volatile keyword on the value in order to guarantee visibility of the change?

Here is my example:

volatile int value = 100000; // <-- do I need the volitile keyword
// ....

public void AnotherThreadMethod()
{
 while(Interlocked.Decrement(ref value)>0)
 {
  // do something
 }
}


public void AThreadMethod()
{
 while(value > 0)
 {
  // do something
 }
}

Update:
I was a bad sport and I changed the original example, so here it is again:

public class CountDownLatch
{
    private volatile int m_remain; // <--- do I need the volatile keyword here?
    private EventWaitHandle m_event;

    public CountDownLatch(int count)
    {
        Reset(count);
    }

    public void Reset(int count)
    {
        if (count < 0)
            throw new ArgumentOutOfRangeException();
        m_remain = count;
        m_event = new ManualResetEvent(false);
        if (m_remain == 0)
        {
            m_event.Set();
        }
    }

    public void Signal()
    {
        // The last thread to signal also sets the event.
        if (Interlocked.Decrement(ref m_remain) == 0)
            m_event.Set();
    }

    public void Wait()
    {
        m_event.WaitOne();
    }
}
+2  A: 

Them do not need* volatility, because you never check the value of the interlocked variable. Instead you always check the value returned by the interlocked operation(s). Mixing interlocked operations and ordinary assignment/comparison always result in incorrect code.

I'm not sure what the Reset() function intent is, but that piece of code has no place in inter-thread primitive: you assign to m_remain, you check the value of m_remain directly, is pretty bad. I strongly suggest you take it out: not only is implemented incorrectly, but I highly doubt the semantics of 'resetting' the counter mid-life-span are needed. Leave it simple: ctor (move the code from Reset into it) Signal and Wait are the only three operators needed, and they are correct as they are now.

Updated After you edited the code.

Ignoring the fact that you shouldn't mix the two, if you do end up mixing them then yes, volatile is still needed. Volatile is primarily about the IL code and the JIT code generated to make sure the value is always read from the actual memory location and no optimization occurs, like code reordering. The fact that an unrelated piece of code updates the value using Interlocked operations play no effect on other parts that read the value. W/o a volatile attribute, the compiler/JIT may still generate code that ignores the writes that occur somewhere else, irrelevant if the writes are interlocked or direct assignment.

BTW, there are valid patterns that mix ordinary read and interlocked operations, but they usually involve Interlocked.CompareExchange and the go like this: read current state, do some computation based on current state, attempt to replace state as an interlocked compare-exchange: if succeed fine, if not drop the computation result and go back to step 1.

Remus Rusanu
Well, my post made sense before your edit lol
Remus Rusanu
@Remus Sorry, I put the example back up... I really didn't mean to trick you! :)
Lirik
@Remus So out goes the Reset function... it really didn't make any sense. I thought it might be useful for reusing the same latch, but it adds more confusion than anything else.
Lirik
I though that the purpose of Reset. It would be perfectly fine if you know for sure is only called in a safe context, but you can never know. Is much easier to provide an API that devs cannot misuse, rather than provide an API that can be misused and ask users to be carefull. Noone reads the manual...
Remus Rusanu
@Remus thanks for the clarifications and again sorry for the multiple edits...
Lirik
A: 

I think System.Threading.Thread.VolatileRead(ref myVariable) might be what you are looking for. Used in conjunction with Interlocked.Increment it can be used to guarantee that changes are atomic and the values that you read are the very latest.

open-collar