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 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
_instance
is inside the lock, only one thread can write to_instance
at a time (and invokeGetName()
) - 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 asvolatile
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 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.