2

All around the 'net there's advice against doing this in C#:

readonly object _lock = new object();
string _instance = null;
 
public string Instance {
    get {
        if (_instance == null) {
            lock (_lock) {
                if (_instance == null) _instance = GetName();
            }
        }
        return _instance;
    }
}

Everywhere says you need to at least mark _instance as volatile to make this thread-safe (or just use Lazy<T>, but that's besides-the-point).

However, I've never really understood why this is. I've never been able to actually replicate a degenerate behaviour, and although that doesn't prove anything, in my head I see the following:

  • The lock statement compiles down to Monitor.Enter()/Monitor.Exit() calls which themselves emit full memory barriers, meaning reads/writes can not cross the lock() boundary
  • Because the second check and write to _instance is inside the lock, only one thread can write to _instance at a time (and invoke GetName())
  • Because Monitor.Exit() emits a full fence, by the time the _lock is 'marked' as open for any other thread to acquire it, _instance must have been written from their point-of-view, regardless of whether it is marked as volatile or not
  • And furthermore, I actually got Microsoft to clarify that making a field volatile only acts as a shorthand for wrapping writes/reads to it in Volatile.Read()/Volatile.Write() and does nothing to "ensure immediacy/up-to-date values", whatever that would even mean.

The only problem I can maybe think of is premature publication of a constructed object (if GetName() was instead creating a new object); is that the issue? But even then, I'm not sure if that's valid in C#, and I don't see how making _instance volatile would help anyway.

Xenoprimate
  • 7,691
  • 15
  • 58
  • 95
  • why the downvote? seems like a good question. – Keith Nicholas Jul 06 '20 at 02:25
  • Hi yes, I'm actually just reading through that. For some reason it didn't show me that as a related item until I submitted the question- I'll vote to close it myself as duplicate in a few minutes if it does answer my question. Edit: Voted, please close! Thanks :) – Xenoprimate Jul 06 '20 at 02:37
  • There are a bunch of historical duplicates and a wealth of information on the topic. In short this should be fine in a lot of cases, however there are cases platforms environments where its not so clear cut. If you want this to be bullet-proof you know the answer – TheGeneral Jul 06 '20 at 02:41

0 Answers0