Is it correct to perform regular reads on a field lazily-initialized by Interlocked.CompareExchange?

Question

Suppose you have a property public Foo Bar { get; } that you want to lazily initialize. One such approach might be to use the Interlocked class, which guarantees atomicity for certain sequences of operations (e.g. incrementing, adding, compare-exchange). You could do this:

private Foo _bar;

public Foo Bar
{
    get
    {
        // Initial check, necessary so we don't call new Foo() every time when invoking the method
        if (_bar == null)
        {
            // In the unlikely case 2 threads come in here
            // at the same time, new Foo() will simply be called
            // twice, but only one of them will be set to _bar
            Interlocked.CompareExchange(ref _bar, new Foo(), null);
        }
        return _bar;
    }
}

There are many places that demonstrate this approach to lazy initialization, e.g. this blog and places in the .NET Framework itself.

My question is, shouldn't the read from _bar be volatile? For example Thread 1 could have called CompareExchange, setting the value of _bar, but that change wouldn't be visible to Thread 2 since (if I understand this question correctly) it may have cached the value of _bar as null, and it may end up calling Interlocked.CompareExchange again, despite the fact that Thread 1 already set _bar. So shouldn't _bar be marked volatile, to prevent this from happening?

In short, is this approach or this approach to lazy initialization correct? Why is Volatile.Read (which has the same effect as marking a variable volatile and reading from it) used in one case, but not the other?

edit TL;DR: If one thread updates the value of a field via Interlocked.CompareExchange, will that updated value be immediately visible to other threads performing non-volatile reads of the field?

Answer 1

My first thought is "who cares?" :)

What I mean is this: the double-check initialization pattern is almost always overkill and it's easy to get wrong. Most of the time, a simple lock is best: it's easy to write, performant enough, and clearly expresses the intent of the code. Furthermore, we now have the Lazy<T> class to abstract lazy initialization, further removing any need for us to manually implement code to do it.

So, the specifics of the double-check pattern aren't really all that important, because we shouldn't be using it anyway.

That said, I would agree with your observation that the read ought to be a volatile read. Without that, the memory barrier provided by Interlocked.CompareExchange() won't necessarily help.

This is mitigated by two things though:

1. The double-check pattern isn't guaranteed anyway. There's a race condition even if you have a volatile read, so it has to be safe to initialize twice. So even if the memory location is stale, nothing truly bad will happen. You'll call the Foo constructor twice, which isn't great but it can't be a fatal problem, because that could happen anyway.
2. On x86, memory access is volatile by default. So it's really only on other platforms that this becomes an issue anyway.