3

I have a ConcurrentDictionaryWrapper class that wraps ConcurrentDictionary<K,V> and implements the IProducerConsumerCollection interface so I can use it with a BlockingCollection. Producers will add values to the BlockingCollection with a key. The idea is that if the key exists, we replace the value in the underlying dictionary. My ConcurrentDictionaryWrapper.TryAdd() method is as follows:

public bool TryAdd(KeyValuePair<TKey, TValue> item)
{
    _wrapped[item.Key] = item.Value
    return true;
}

The problem I'm seeing is that if the value is replaced, the BlockingCollection will see this as an addition.

var wrapper = new ConcurrentDictionaryWrapper<string, object>();
var bc = new BlockingCollection<KeyValuePair<string, object>>(wrapper);
bc.TryAdd(new KeyValuePair<string, object>("key", new object()));
bc.TryAdd(new KeyValuePair<string, object>("key", new object()));
wrapper.Count; # returns 1
bc.Count; # returns 2

I cannot return false in TryAdd() above because the BlockingCollection will raise an InvalidOperationException.

Is there a way to achieve the behaviour I want? I want this to be as simple as possible but there doesn't seem to be a way to have this "AddOrUpdate" behaviour with a BlockingCollection by just implementing IProducerConsumerCollection. I want to avoid having to TryTake() before calling TryAdd() on the BlockingCollection — I would probably just use a standard lock around a Dictionary and synchronise the producers/consumers with AutoResetEvent.

kenkam
  • 55
  • 1
  • 6

2 Answers2

1

Maybe, instead of replacing, flagging previous item as invalid might be better suited. Keep a boolean flag with each value. When item with existing key is added, find the previous item, set it's flag to false and add new item with flag set to true.

Then, when the consumer gets the item, it will check if it is valid. If not, it will throw it away and request new one.

Euphoric
  • 12,645
  • 1
  • 30
  • 44
  • This would work but I guess it wouldn't be as efficient as using a simple lock for data access and the event primitives to sync threads? – kenkam Dec 18 '13 at 13:56
1

There's considerable friction here, BlockingCollection<> maintains its own count and doesn't use your IProducerConsumerCollection.Count implementation. So since you didn't fail the TryAdd(), the BlockingCollection is now convinced that there are two items in the collection. Even though you could only ever retrieve one of them. So you certainly cannot leave it the way it is, your code will always deadlock.

What you need is a BlockingCollection method that returns false or silently fails if the item is already present. But it doesn't have one, the InvalidOperationException is unavoidable. You could catch it but that's butt-ugly.

The alternative would be a Contains() method that you'd use to avoid calling TryAdd() if it returns true. But now the collection isn't thread-safe anymore. Which is why BlockingCollection doesn't have that method.

This is a rock and a hard place, you cannot make it work as envisioned. You'll have to spin your own. Best to steal the code of the threading masters, the bounded buffer in this magazine article should be a good start to get the lock design correct.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Thanks for the answer, this confirmed my suspicions. I was misled by http://stackoverflow.com/questions/5843383/what-type-of-iproducerconsumercollectiont-to-use-for-my-task -- I think the example there works because none of the items are being 'updated' as they are all being consumed by the main thread before they could be updated. – kenkam Dec 18 '13 at 15:59