CODEWITHSUNDEEP
c#multithreadingthread-safety
Juliet
Juliet

Reputation: 81526

Threadsafe Lazy Class

I have class Lazy which lazily evaluates an expression:

public sealed class Lazy<T>
{
    Func<T> getValue;
    T value;

    public Lazy(Func<T> f)
    {
        getValue = () =>
            {
                lock (getValue)
                {
                    value = f();
                    getValue = () => value;
                }
                return value;
            };
    }

    public T Force()
    {
        return getValue();
    }
}

Basically, I'm trying to avoid the overhead of locking objects after they've been evaluated, so I replace getValue with another function on invocation.

It apparently works in my testing, but I have no way of knowing if it'll blow up in production.

Is my class threadsafe? If not, what can be done to guarantee thread safety?

Upvotes: 1

Views: 808

Answers (4)

Konrad Rudolph
Konrad Rudolph

Reputation: 545618

Can’t you just omit re-evaluating the function completely by either using a flag or a guard value for the real value? I.e.:

public sealed class Lazy<T>
{
    Func<T> f;
    T value;
    volatile bool computed = false;
    void GetValue() { lock(LockObject) { value = f();  computed = true; } }

    public Lazy(Func<T> f)
    {
        this.f = f;
    }

    public T Force()
    {
        if (!computed) GetValue();
        return value;
    }
}

Upvotes: 2

Daniel
Daniel

Reputation: 16439

Your code has a few issues:

  1. You need one object to do the locking on. Don't lock on a variable that gets changed - locks always deal with objects, so if getValue is changed, multiple threads might enter the locked section at once.

  2. If multiple threads are waiting for the lock, all of them will evaluate the function f() after each other. You'd have to check inside the lock that the function wasn't evaluated already.

  3. You might need a memory barrier even after fixing the above issues to ensure that the delegate gets replaced only after the new value was stored to memory.

However, I'd use the flag approach from Konrad Rudolph instead (just ensure you don't forget the "volatile" required for that). That way you don't need to invoke a delegate whenever the value is retrieved (delegate calls are quite fast; but not they're not as fast as simply checking a bool).

Upvotes: 2

Adam Robinson
Adam Robinson

Reputation: 185643

This looks more like a caching mechanism than a "lazy evaluation". In addition, do not change the value of a locking reference within the lock block. Use a temporary variable to lock on.

The wait you have it right now would work in a large number of cases, but if you were to have two different threads try to evaluate the expression in this order:

Thread 1
Thread 2
Thread 1 completes

Thread 2 would never complete, because Thread 1 would be releasing a lock on a different reference than was used to acquire the lock (more precisely, he'd be releasing a nonexistent lock, since the newly-created reference was never locked to begin with), and not releasing the original lock, which is blocking Thread 2.

While I'm not entirely certain what this would do (aside from perform a synchronized evaluation of the expression and caching of the result), this should make it safer:

public sealed class Lazy<T>
{
    Func<T> getValue;
    T value;
    object lockValue = new object();

    public Lazy(Func<T> f)
    {
        getValue = () =>
            {
                lock (lockValue)
                {
                    value = f();
                    getValue = () => value;
                }
                return value;
            };
    }

    public T Force()
    {
        return getValue();
    }
}

Upvotes: 0

Adam Maras
Adam Maras

Reputation: 26863

I'm not entirely sure what you're trying to do with this code, but I just published an article on The Code Project on building a sort of "lazy" class that automatically, asynchronously calls a worker function and stores its value.

Upvotes: 0

Related Questions