2

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?

Community
  • 1
  • 1
James Ko
  • 32,215
  • 30
  • 128
  • 239

2 Answers2

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.
Peter Duniho
  • 68,759
  • 7
  • 102
  • 136
  • Thanks for answering! About your first point: if the thread that does a regular read of the variable sees it as null and caches the result, then won't return _bar end up returning the cached value (null), aka return null? (Also .NET is actually very common on ARM processors, e.g. Windows Phone, which is why I'm concerned about this.) – James Ko Jul 11 '16 at 03:19
  • Sorry, can you clarify? Are you assuming the variable is enregistered and so returns the old `null` value, even after calling `CompareExchange()`? If so, I don't think that's legal. Even if the variable is enregistered, having called `CompareExchange()` the compiler will necessarily be required to read the value again before returning, because `CompareExchange()` may have changed its value. If that's not what you mean, could you be more specific about what you mean? – Peter Duniho Jul 11 '16 at 03:45
  • That was what I meant. So to be clear, even if one thread does not see the value of a field set by another thread using `CompareExchange`, if it calls `CompareExchange` itself it has to re-load the value from main memory on the next read? – James Ko Jul 12 '16 at 04:33
  • Yes, that's exactly what I mean. The first access, prior to calling `CompareExchange()` could well get a stale value. But after calling `CompareExchange()`, the value must be up-to-date at that point. For the method to work at all, it has to execute a volatile read on the memory location and that alone will ensure the value is up-to-date, even before the comparison happens. – Peter Duniho Jul 12 '16 at 06:02
1

In this particular case doing non-volatile read won't make the code incorrect because even if the second thread's missed update of _bar it will observe it on CompareExchange. Volatile read (potentially) allows to see the updated value earlier without necessity to do heavyweight CompareExchange.

In other cases a memory location written via Interlocked, Volatile.Write, or inside lock region must be read as volatile.

OmariO
  • 506
  • 4
  • 11
  • Thank you for answering! Just curious since I'm not so experienced with threading, is the reason for this since `CompareExchange` [does a memory barrier](http://stackoverflow.com/a/1716587/4077294) which forces the second thread to reload the value from memory, or something along those lines? – James Ko Jul 22 '16 at 05:23
  • If you are asking this question: `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?` Then no, `CompareExchange` doesn't make the change _immediately_ visible to the code executed by other threads. Values at memory locations can be cached on many levels - it is not only about CPU. So on other threads you need a barrier, at least load-acquire which is volatile read in .NET terms. – OmariO Jul 25 '16 at 10:22
  • @JamesKo IMO more convenient and **practical** model to use when dealing with memory accesses is not caching or visibility, but memory operations reordering. Have a look at [this good comment](http://stackoverflow.com/a/17631001/2387098). – OmariO Jul 25 '16 at 18:23
  • Thank your for the link! Regarding your first comment, I meant if the second thread does not see the updated value (the null check returns true, although it's already initialized in memory) and the second thread calls `CompareExchange` again, are memory barriers the reason it'll see it on the second read? – James Ko Jul 26 '16 at 02:35
  • 1
    @JamesKo yes, as `CompareExchange` is read/write with full memory barrier – OmariO Jul 27 '16 at 06:37