No, this code is not atomic - if Items
is accessed from multiple threads in parallel, _items
may actually get created more than once and different callers may receive a different value.
This code needs locking because it first performs a read, a branch and a write (after an expensive deserialization call). The read and the write by themselves are atomic but - without a lock - there's nothing to prevent the system to switch to another thread between the read and the write.
In pseudo(ish) code, this is what may happen:
if (_items==null)
// Thread may be interrupted here.
{
// Thread may be interrupted inside this call in many places,
// so another thread may enter the body of the if() and
// call this same function again.
var s = ConfigurationManager.AppSettings.get_Item("Items");
// Thread may be interrupted inside this call in many places,
// so another thread may enter the body of the if() and
// call this same function again.
var i = JsonConvert.DeserializeObject(s);
// Thread may be interrupted here.
_items = i;
}
// Thread may be interrupted here.
return (_items);
This shows you that without locking it's possible for multiple callers to get a different instance of the Items
list.
You should look into using Lazy<T>
which will make this sort of initialization a lot simpler and safe.
When should I use Lazy<T>?
Also, keep in mind that List<T>
itself is not thread-safe - you may want to use a different type (like ConcurrentDictionary<T1, T2>
or ReadOnlyCollection<T>
) or you may need to use locking around all operations against this list.
Rob, in the comments, pointed out that the question may be about whether a given assignment is atomic - a single assignment (that is a single write) of a reference is guaranteed to be atomic but that doesn't make this code safe because there's more than a single assignment here.