ConcurrentDictionary.GetOrAdd - Add only if not null

Question

I'm using ConcurrentDictionary to cache data with parallel access and sometimes new items can be stored in db and they are not loaded into cache. This is reason why I use GetOrAdd

public User GetUser(int userId)
{
    return _user.GetOrAdd(userId, GetUserFromDb);        
}

private User GetUserFromDb(int userId)
{
    var user = _unitOfWork.UserRepository.GetById(userId);

    // if user is null, it is stored to dictionary

    return user;
}

But how I can check if user was get from db and store user to dictionary only if user is not null?

Possibly I can remove null from ConcurrentDictionary immediately after GetOrAdd but it doesn't look thread safe and it is not very elegant solution. Useless insert and remove from dictionary. Do you have any idea how to do it?

Answer 1

Here's a hacky solution, I hope something better is possible. Make GetUserFromDb throw if the user is not found. This aborts the store into the dictionary. Make GetUser catch the exception. This is using exceptions for control flow which is not nice.

Answer 2

public User GetUser(int userId)
{
    var user = _user.GetOrAdd(userId, GetUserFromDb);
    if (user == null) _user.TryRemove(userId, out user);    
}

You can also wrap that into an extension method:

public static TValue GetOrAddIfNotNull<TKey, TValue>(
    this ConcurrentDictionary<TKey, TValue> dictionary,
    TKey key, 
    Func<TKey, TValue> valueFactory) where TValue : class
{
    var value = dictionary.GetOrAdd(key, valueFactory);
    if (value == null) dictionary.TryRemove(key, out value);
    return value;
}

Then your code will look like:

public User GetUser(int userId)
{
    var user = _user.GetOrAddIfNotNull(userId, GetUserFromDb)   
}

UPDATE

As per @usr comment, there might be a case when:

1. Thread 1 executes GetOrAdd, adds null to the dictionary and pauses.
2. User is added to the database.
3. Thread 2 executes GetOrAdd and retrieves null from the dictionary instead of hitting the database.
4. Thread 1 and Thread 2 execute TryRemove and remove record from the dictionary.

With this timing, Thread 2 will get null instead of hitting the database and getting the user record. If this edge case matters to you and you still want to use ConcurrentDictionary, then you can use lock in the extension method:

public static class ConcurrentDictionaryExtensions
{
    private static readonly object myLock = new object();

    public static TValue GetOrAddIfNotNull<TKey, TValue>(
        this ConcurrentDictionary<TKey, TValue> dictionary,
        TKey key, 
        Func<TKey, TValue> valueFactory) where TValue : class
    {
        lock (myLock)
        {
            var value = dictionary.GetOrAdd(key, valueFactory);
            if (value == null) dictionary.TryRemove(key, out value);
            return value;
        }
    }
}

Answer 3

I am extending @NikolaiSamteladze solution to include double-checked locking so that other threads can skip acquiring lock after dictionary updation

public static class ConcurrentDictionaryExtensions
{
    private static readonly object myLock = new object();

    public static TValue GetOrAddIfNotNull<TKey, TValue>(
        this ConcurrentDictionary<TKey, TValue> dictionary,
        TKey key,
        Func<TKey, TValue> valueFactory) where TValue : class
    {
        TValue value;
        if (!dictionary.TryGetValue(key, out value))
        {
            lock (myLock)
            {
                value = dictionary.GetOrAdd(key, valueFactory);
                if (value == null) dictionary.TryRemove(key, out value);
            } 
        }
        return value;
    }
}