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
lockstatement compiles down toMonitor.Enter()/Monitor.Exit()calls which themselves emit full memory barriers, meaning reads/writes can not cross thelock()boundary - Because the second check and write to
_instanceis inside the lock, only one thread can write to_instanceat a time (and invokeGetName()) - Because
Monitor.Exit()emits a full fence, by the time the_lockis 'marked' as open for any other thread to acquire it,_instancemust have been written from their point-of-view, regardless of whether it is marked asvolatileor not - And furthermore, I actually got Microsoft to clarify that making a field
volatileonly acts as a shorthand for wrapping writes/reads to it inVolatile.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.