c# How to create a thread safe, two dimensional lock collection?

Question

I'm trying to implement a thread safe, two dimensional collection to control access to multiple resources. The problem is from what I can see I need to perform locking at both the collection level (which is simple enough) and at the individual object level, but not necessarily both at the same time which could potentially cause some problems.

public class LockStore
{
    private object _syncLock = new object();
    private Dictionary<string, StoredLock> _locks = new Dictionary<string, StoredLock>();

    public StoredLock GetOrAdd(string id, StoredLock lockDetails)
    {
        if (!_locks.ContainsKey(id))
            lock (_syncLock)
            {
                if (!_locks.ContainsKey(id))
                    _locks.Add(id, lockDetails);
            }
        return _locks[id];
    }

    public void Remove(string id)
    {
        lock (_syncLock)
        {
            _locks.Remove(id);
        }
    }
}

public class StoredLock
{
    public string Id { get; set; }
    public DateTime CreatedDateTime { get; set; }
}

Say for example, a thread could call into the LockStore.GetOrAdd() method, returning a StoredLock object. I'd then like to lock the StoreLock object to prevent other threads from being able to access it, but without holding the lock for the entire dictionary.

var id = "someId"
var storedLock = lockStore.GetOrAdd(id, new StoredLock());

lock (storedLock)
{
    //Do something
}

And at some point in a different thread entirely...

lockStore.Remove(id);

At the point the lock is acquired on the StoredLock object stored within the LockStore, the lock is not held for the LockStore so theoretically LockStore.Remove() could be invoked for that particular Id as the Remove() method has no understanding of the locks held on the objects it actually contains. I'm not sure what would happen in this circumstance but I don't think it would be good!

Also, I think this could potentially violate the design principle that objects used for locking aren't publicly accessible?

I'm really at a loss with this, so any suggestions re: solving the problem or the actual approach would be very much appreciated.

EDIT: to be particularly specific - I do not want to hold the lock on the LockStore collection and the individual StoredLock at the same time as this would slow down the processing considerably.

Did you take a look at [`ConcurrentDictionary`](https://msdn.microsoft.com/en-us/library/dd287191(v=vs.110).aspx)? — Yacoub Massad, Jan 08 '16 at 12:49
What problem are you trying to solve? I don't get it. Sounds like XY problem for me — Sriram Sakthivel, Jan 08 '16 at 12:50
@YacoubMassad, yes, I could use ConcurrentDictionary for the lock store but that wouldn't solve the problem of two dimensional locking from what I understand. — James Law, Jan 08 '16 at 12:50
@SriramSakthivel - I need to hold a lock on an object based on it's Id to prevent concurrent processing. There are many possible Id's so locking the collection storing them whilst processing each Id would cause performance issues. — James Law, Jan 08 '16 at 12:54
Why would you even want something like this? Why don't you want to use locks inside methods of those objects? I mean, why don't you want to make your classes thread safe instead of trying to use them that way? I believe, it is much easier to control object lifetime rather than access to it. Dictionary of lock objects looks complicated and totally unsafe for me. — THTP, Jan 08 '16 at 13:14
@THTP - I agree - I don't like this solution much either and it's not fit for purpose as it is at the moment. But I can't figure out how to do it any other way. The issue is with multiple threads that have no knowledge of one another, calling multiple Ids (which could be the same) and actually preventing the same processing for the same ids on those threads at the same time, whilst recording when the processing was last completed. — James Law, Jan 08 '16 at 13:24
Why is it dangerous to remove the item from the dictionary while another thread is operating on the item? — Yacoub Massad, Jan 08 '16 at 13:32
@YacoubMassad thread one could be in the middle of processing, thread 2 could remove it from the dictionary (thread 1 would still have the lock on the object until it finished processing, which wouldn't be fatal) however if thread 3 came along, saw the id wasn't in the dictionary, it too would start the same processing that thread 1 was currently in the process of. — James Law, Jan 08 '16 at 13:42
@HenkHolterman it's a hypothetical - it wouldn't do it deliberately but as they're running in separate threads they have no knowledge of one another's actions and thus wouldn't be safe. — James Law, Jan 08 '16 at 13:45
@HenkHolterman - the Ids that are requested aren't determined by the program, they're determined by the requests to the program. Think of a user sending http requests with different querystring values. — James Law, Jan 08 '16 at 13:58
What should happen if thread 2 tries to remove it from the dictionary while thread 1 is in middle of processing? Should it block until thread 1 release the item or should it simply return `false`? — Yacoub Massad, Jan 08 '16 at 13:59
@YacoubMassad - good question! Ideally it should return false. — James Law, Jan 08 '16 at 14:01

Yacoub Massad · Accepted Answer · 2016-01-08T14:24:56.183

After understanding the problem, I think that you should do something like this:

public class LockStore
{
    private readonly Dictionary<string, StoredLock> m_Dictionary =
        new Dictionary<string, StoredLock>();

    public void UseResource(string resource_id, Action<StoredLock> action)
    {
        StoredLock stored_lock = null;

        lock(m_Dictionary)
        {
            if (m_Dictionary.ContainsKey(resource_id))
            {
                stored_lock = m_Dictionary[resource_id];
            }
            else
            {
                stored_lock = new StoredLock
                {
                    Id = resource_id,
                    CreatedDateTime = DateTime.Now
                };

                m_Dictionary.Add(resource_id,stored_lock);
            }
        }

        lock(stored_lock)
        {
            action(stored_lock);
        }
    }

    public bool RemoveLock(string resource_id)
    {
        lock (m_Dictionary)
        {
            if (!m_Dictionary.ContainsKey(resource_id))
                return true;

            var stored_lock = m_Dictionary[resource_id];

            bool taken = false;
            Monitor.TryEnter(stored_lock, ref taken);

            if (!taken)
                return false;

            try
            {
                m_Dictionary.Remove(resource_id);
            }
            finally
            {
                Monitor.Exit(stored_lock);
            }

            return true;
        }
    }
}

This code is not perfectly designed but it works. You should work on it to make it better.

Basically, this class allows you to do something like this from Thread 1:

lockStore.UseResource("resource1", sl => 
{
    //Do some processing with s1
});

And to do something like this from thread 2:

var removed = lockStore.RemoveLock("resource1");

RemoveLock will return immediately and give you a return value of false if some thread is still holding the lock on the object.

c# How to create a thread safe, two dimensional lock collection?

1 Answers1