Initialization of thread-safe lazy initialization; is changing Func <> a good idea?
The class at the bottom is an implementation of a fast, thread-safe, "non-blocking", lazy initializer.
I say "lock free", although it's not really; It uses the lock until it is initialized and then replaces the call to get data with data that does not use the lock. (Similar to a double checked mutable lock and from performance tests I get similar results.)
My question is, is this a good / safe thing?
(After I wrote this, I noticed that .net 4.0 has LazyInit<T>
one that does the same operations and more, but in a very contrived example I created, my implementation was slightly faster :-))
NB The class was changed to include a member Value
and changed to_get
class ThreadSafeInitializer<TOutput> where TOutput : class { readonly object _sync = new object(); TOutput _output; private volatile Func<TOutput> _get; public TOutput Value { get { return _get(); } } public ThreadSafeInitializer(Func<TOutput> create) { _get = () => { lock (_sync) { if (_output == null) { _output = create(); _get = () => _output; // replace the Get method, so no longer need to lock } return _output; } }; } }
source share
As Guffa said, you want to add volatile to Get to make it safe
However, I found the implementation a bit tricky. I think he's trying to be too fantastic for his own good.
So it's easiest to follow these lines:
class Lazy<T> { readonly object sync = new object(); Func<T> init; T result; volatile bool initialized; public Lazy(Func<T> func) { this.init = func; } public T Value { get { if(initialized) return result; lock (sync) { if (!initialized) { result = this.init(); initialized = true; } } return result; } } }
Minor benefits:
- Less fantasy
- Lack of performance when calling Func
- Locks only on initialization.
- Accessing the value through the obj.Value object is a little more intuitive than obj.Get ()
source share
I write this code a lot:
private SomeThing _myThing = null; public SomeThing MyThing { get { return _myThing != null ? _myThing : _myThing = GetSomeThing(); } }
Light loads are just fine and very readable. My point? the absence of a lock (object) is not accidental. Lazy loading properties with locks will ask IMO problems.
The use of the lock operator is to be tightly controlled, and both Paul and Sam state that they are accused of invoking a code they did not write down while holding the lock. Maybe it doesn't matter, maybe it will slow down your program. And are you doing this from within an innocent looking property so you can save a few milliseconds?
I guess if you can safely load it twice without disastrous results, that would be better. Then in the rare case that two threads get the same property at the same time, then two instances are loaded ... Depends on what your loading is, if it's really bad, I bet it doesn't matter most of the time that each the thread gets another instance. If it matters, I would recommend doing the necessary locks while building and not bothering yourself with lazy loading. Probably since a constructor can only happen on one thread, you won't need a lock at all.
source share