Can't safely lock a value of a ConcurrentDictionary

Question

Hi,

I'm having trouble locking on an item within a Collection - specifically a ConcurrentDictionary.

I need to accept a message, look up that message within the Dictionary and then run a lengthy scan on that. As the program takes a lot of memory, after the scan the objects return true if they think its a good time to delete it (which I do by removing it from the Dictionary). However, another thread could come at a similar time and try to access that same object right after the delete. This is my first attempt:

string dictionaryKey = myMessage.someValue;

DictionaryObject currentObject = myConcurrentDictionary.GetOrAdd(dictionaryKey, new DictionaryObject());
// we can be interrupted here
lock (currentObject)
{
    //KeyNotFoundException is possible on line below
    if (myConcurrentDictionary[dictonaryKey].scan(myMessage)) // Scans the message - returns true if the object says its OK to remove it from the dictionary
    {
      DictionaryObject temp;                      //   It's OK to delete it
      if (!queuedMessages.TryRemove(ric, out temp))   // Did delete work?
       throw new Exception("Was unable to delete a DictionaryObject that just reported it was ok to delete it");
    }
}

However, the above doesn't work - it's possible for one thread to remove an object from the Dictionary right before another is going to attempt to access that object within the Dictionary. After reading that lock is shorthand for Monitor.Enter and Monitor.Exit, I tried this:

string dictionaryKey = myMessage.someValue;
Monitor.Enter(GetDictionaryLocker);
DictionaryObject currentObject = myConcurrentDictionary.GetOrAdd(dictionaryKey, new DictionaryObject());
// we can be interrupted here
lock (currentObject)
{
    Monitor.Exit(GetDictionaryLocker);
    //KeyNotFoundException is still possible on line below
    if (myConcurrentDictionary[dictonaryKey].scan(myMessage)) // Scans the message - returns true if the object says its OK to remove it from the dictionary
    {
      DictionaryObject temp;                   //   It's OK to delete it
      if (!queuedMessages.TryRemove(ric, out temp))   // Did delete work?
       throw new Exception("Was unable to delete a DictionaryObject that just reported it was ok to delete it");
    }
}

Both ways can result in a KeyNotFoundException when trying to look the object up within the Dictionary.

Does anyone know how I could find the object I want to lock and then lock it without being interrupted? Sorry - I'm new at concurrency and feel thoroughly confused!

Thanks,

Frederik

Answer 1

I think there is a fundamental misunderstanding of "locking" an object here.

When you hold a lock on an object (Monitor.Enter), it doesn't actually prevent other threads from using that object. It just prevents other threads from taking a lock on that specific object. What you need to do is add a secondary object, and lock on it - and make sure every thread locks on it as well.

That being said, this is going to introduce synchronization overhead. It might be worth trying to come up with a design where the lock is only used if there is a conflict - something like actually removing the object prior to scan, then locking, and holding the lock during the scan. Other threads can only try to obtain the lock if the object they are requesting is not part of the dictionary...

Answer 2

You should remove the object from the dictionary before you start your scan, to prevent any other thread from trying to use it concurrently. You can always add it back in if you have to later on, after a failure in scan(). Both remove and add are guaranteed thread-safe on this concurrent collection.

This should make what you want possible without any locks or Monitor usage.

string dictionaryKey = myMessage.someValue;

DictionaryObject currentObject = null;
if (myConcurrentDictionary.TryRemove(dictionaryKey, out currentObject))
{
    //KeyNotFoundException is possible on line below
    if (!currentObject.scan(myMessage)) // Scans the message - returns true if the object says its OK to remove it from the dictionary
    {
      if (!myConcurrentDictionary.TryAdd(dictionaryKey, currentObject))
       throw new Exception("Was unable to re-insert a DictionaryObject that is not OK for deletion");
    }
} 

My concern with this, without understanding the rest of your code, is whether some other thread can add back in another message with the same key during your call to scan(). This will cause TryAdd to fail. If this is a possibility, more work is needed.

The problem with your current model is that even though the collection is thread-safe, what you really must do if you wish to leave the 'being scanned' items in the collection is to do the following combination of operations atomically: 1. find a free item and 2. mark it as 'in use'.

1. can be done by virtue of the collection being thread-safe but
2. has to be done separately, so you open up a window for multiple scan()s on the same object.

Answer 3

This is speculation ...

The problem is that the concurrentDictionary returns Unique objects to each thread to prevent them from interfering with each other.

Any add, change or remove methods will make sure that the correct item is accessed but the lock will only lock you local wrapper.

Any solution depends on what you can do with the dictionary.

Could the thread pull the message from the dictionary directly and only replace it after scan, or could you use a master thread that goes over the dictionary and adds all messages to a queue object that the threads pull from and after processing remove documents that should be removed.

That way each thread will never contend for the same message.

Answer 4

How about including a field in the object which indicates what thread owns it, and using Threading.Interlocked.CompareExchange to attempt to acquire it? Figure that if an object is in use, code shouldn't lock but simply abandon the operation.