I have several cases where I use ConcurrentDictionary<TKey, TValue>
for caching of values, but often times I need to perform validation of the value to decide whether to add it to the cache using ConcurrentDictionary<TKey, TValue>.GetOrAdd(TKey, Func<TKey, TValue>)
.
Typically along the lines of:
private readonly ConcurrentDictionary<Type, ISomeObject> someObjectCache =
new ConcurrentDictionary<Type, ISomeObject>();
public ISomeObject CreateSomeObject(Type someType)
{
return someObjectCache.GetOrAdd(someType, type =>
{
if(!Attribute.IsDefined(someType, typeof(SomeAttribute))
// Do something here to avoid the instance from being added to
// `someObjectCache`
ISomeObject someObject;
// Typical factory functionality goes here
return someObject;
});
}
The way I handle this today is to throw an exception which appears to work fine, but I'd like a cleaner approach (maybe a flag I can set or a specific value I can set the return value to) to cancel the GetOrAdd
from within the lambda (though it could realistically be replaced by a full blown method).
Based on my experience with other LINQ like methods, returning null
will result in the value getting added without being checked as such (and reading the IL for GetOrAdd
it looks like it'll result in the same problem), so I don't think that'll work.
Is there some way I can avoid using exceptions to cancel the add using GetOrAdd
?
From what I've read, there's no guarantee that the Add factory method will only be called a single time amongst all callers to Get for the same key.
The relevant portion from that page is at the bottom, quoted here:
Also, although all methods of ConcurrentDictionary(Of TKey, TValue) are thread-safe, not all methods are atomic, specifically GetOrAdd and AddOrUpdate. The user delegate that is passed to these methods is invoked outside of the dictionary's internal lock. (This is done to prevent unknown code from blocking all threads.) Therefore it is possible for this sequence of events to occur:
1) threadA calls GetOrAdd, finds no item and creates a new item to Add by invoking the valueFactory delegate.
2) threadB calls GetOrAdd concurrently, its valueFactory delegate is invoked and it arrives at the internal lock before threadA, and so its new key-value pair is added to the dictionary.
3) threadA's user delegate completes, and the thread arrives at the lock, but now sees that the item exists already
4) threadA performs a "Get", and returns the data that was previously added by threadB.
Therefore, it is not guaranteed that the data that is returned by GetOrAdd is the same data that was created by the thread's valueFactory. A similar sequence of events can occur when AddOrUpdate is called.
The way I read this is that even if you're calling some locking in your Add delegate, you're not guaranteed that the value returned from your add is the one which will actually be used.
So, you shouldn't need to add any further locking, and instead could probably use the following pattern:
private ConcurrentDictionary<Type, ISomeObject> someObjectCache =
new ConcurrentDictionary<Type, ISomeObject>();
public ISomeObject CreateSomeObject(Type someType)
{
ISomeObject someObject;
if (someObjectCache.TryGet(someType, out someObject))
{
return someObject;
}
if (Attribute.IsDefined(someType, typeof(SomeAttribute))
{
// init someObject here
someObject = new SomeObject();
return someObjectCache.GetOrAdd(someType, someObject); // If another thread got through here first, we'll return their object here.
}
// fallback functionality goes here if it doesn't have your attribute.
}
Yes, this will result in some potential for new objects to be created potentially multiple times, but callers will all receive the same result, even if multiple are called. Same as GetOrAdd does now.