CODEWITHSUNDEEP
c#
user5669286
user5669286

Reputation:

How to use Lazy to handle concurrent request?

I'm new in C# and trying to understand how to work with Lazy.

I need to handle concurrent request by waiting the result of an already running operation. Requests for data may come in simultaneously with same/different credentials.

For each unique set of credentials there can be at most one GetDataInternal call in progress, with the result from that one call returned to all queued waiters when it is ready

private readonly ConcurrentDictionary<Credential, Lazy<Data>> Cache
= new ConcurrentDictionary<Credential, Lazy<Data>>();

public Data GetData(Credential credential)
{
    // This instance will be thrown away if a cached
    // value with our "credential" key already exists.
    Lazy<Data> newLazy = new Lazy<Data>(
        () => GetDataInternal(credential),
        LazyThreadSafetyMode.ExecutionAndPublication
    );

    Lazy<Data> lazy = Cache.GetOrAdd(credential, newLazy);
    bool added = ReferenceEquals(newLazy, lazy); // If true, we won the race.
    Data data;

    try
    {
       // Wait for the GetDataInternal call to complete.
       data = lazy.Value;
    }
    finally
    {
        // Only the thread which created the cache value
        // is allowed to remove it, to prevent races.
        if (added) {
            Cache.TryRemove(credential, out lazy);
        }
    }

    return data;
}

Is that right way to use Lazy or my code is not safe?

Update:

Is it good idea to start using MemoryCache instead of ConcurrentDictionary? If yes, how to create a key value, because it's a string inside MemoryCache.Default.AddOrGetExisting()

Upvotes: 12

Views: 1918

Answers (2)

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149628

This answer is directed to the updated part of the original question. See @usr answer regarding thread-safety with Lazy<T> and the potential pitfalls.

I would like to know how to avoid using ConcurrentDictionary<TKey, TValue> and start using MemoryCache? How to implement MemoryCache.Default.AddOrGetExisting()?

If you're looking for a cache which has a mechanism for auto expiry, then MemoryCache is a good choice if you don't want to implement the mechanics yourself.

In order to utilize MemoryCache which forces a string representation for a key, you'll need to create a unique string representation of a credential, perhaps a given user id or a unique username?

If you can, you can create an override of ToString which represents your unique identifier or simply use the said property, and utilize MemoryCache like this:

public class Credential
{
    public Credential(int userId)
    {
        UserId = userId;
    }

    public int UserId { get; private set; }
}

And now your method will look like this:

private const EvictionIntervalMinutes = 10;
public Data GetData(Credential credential)
{
    Lazy<Data> newLazy = new Lazy<Data>(
        () => GetDataInternal(credential), LazyThreadSafetyMode.ExecutionAndPublication);

    CacheItemPolicy evictionPolicy = new CacheItemPolicy
    { 
        AbsoluteExpiration = DateTimeOffset.UtcNow.AddMinutes(EvictionIntervalMinutes)
    };

    var result = MemoryCache.Default.AddOrGetExisting(
        new CacheItem(credential.UserId.ToString(), newLazy), evictionPolicy);

    return result != null ? ((Lazy<Data>)result.Value).Value : newLazy.Value;
}

MemoryCache provides you with a thread-safe implementation, this means that two threads accessing AddOrGetExisting will only cause a single cache item to be added or retrieved. Further, Lazy<T> with ExecutionAndPublication guarantess only a single unique invocation of the factory method.

Upvotes: 1

usr
usr

Reputation: 171246

This is correct. This is a standard pattern (except for the removal) and it's a really good cache because it prevents cache stampeding.

I'm not sure you want to remove from the cache when the computation is done because the computation will be redone over and over that way. If you don't need the removal you can simplify the code by basically deleting the second half.

Note, that Lazy has a problem in the case of an exception: The exception is stored and the factory will never be re-executed. The problem persists forever (until a human restarts the app). In my mind this makes Lazy completely unsuitable for production use in most cases.

This means that a transient error such as a network issue can render the app unavailable permanently.

Upvotes: 6

Related Questions