# Dictionary for locking on unique identifiers without leaking memory

The following code is greatly inspired on this question and this answer. That answer works quite well for a specific case, but I was looking for a solution that:

1. could be used more generic (so I can put this in a base library and never have to think about it anymore).
2. got rid of the Find() and replace it with a O1 dictionary lookup. To be honest I doubt if this would have any performance effect, but it was tickling my OCDs.

I've tried to test it with running about 10 threads and superficially it seems to work. But as concurrency is hard to test and reason about - am I missing something? Any other obvious upgrades?

public class LockDictionary<TKey>
{
object _RootLock = new object();
Dictionary<TKey,ItemLock> _KeyToLock = new Dictionary<TKey,ItemLock>();

public IDisposable Lock(TKey key)
{
ItemLock item_lock;

lock  ( _RootLock )
{
item_lock = GetOrCreate(key);
item_lock._Counter++;
}

Monitor.Enter(item_lock);

return new LockCounter(this, key);
}

private ItemLock GetOrCreate(TKey key)
{
if ( !_KeyToLock.TryGetValue(key, out var item_lock) )
{
item_lock = new ItemLock();
}

return item_lock;
}

private void ReleaseLock(TKey key)
{
lock (_RootLock)
{
var item_lock = _KeyToLock[key];
item_lock._Counter--;

if ( item_lock._Counter == 0 )
_KeyToLock.Remove(key);

Monitor.Exit(item_lock);
}
}

private class ItemLock
{
public object   _ItemLock   = new object();
public int      _Counter    = 0;
}

private class LockCounter : IDisposable
{
LockDictionary<TKey> _Parent;
TKey _Key;

public LockCounter(LockDictionary<TKey> parent, TKey key)
{
_Parent     = parent;
_Key        = key;
}

public void Dispose()
{
_Parent.ReleaseLock(_Key);
}
}
}


Update

To clarify; I'm looking for an implementation for the following:

// minimal definition boilerplate
// possibility of using other types as key, like string, long or a struct
static LockDictionary<int> _LockDictionary = new LockDictionary<int>();

// minimal boilerplate for having a separate lock per "groupId"
using ( _LockDictionary.Lock(groupId) )
{
// can only be run once concurrently per unique groupId
}

• Why you're not locking Monitor.Enter(item_lock) with your _RootLock but locking Monitor.Exit(item_lock);? Seems a bit inconsistent to me – Bohdan Stupak Nov 5 '19 at 9:25
• Good point - I guess I can move the Monitor.Exit outside the _RootLock. – Dirk Boer Nov 5 '19 at 9:30
• If it fits your needs I'd rewrite this to use a ConditionalWeakTable, it's probably faster, slimmer and already well tested. Also note that if Lock() fails anywhere you might have orphans in your dictionary (and acquired but unreleased locks and/or inconsistencies between _Counter and calls to Monitor.Enter()). In short...hmmm...instead of trying to have a complex locking mechanism which should be extremely well reviewed (and with good chances to take it wrong), what are you trying to achieve? – Adriano Repetti Nov 5 '19 at 10:33
• Hi @AdrianoRepetti, I've updated the question with the "interface" I'd like to fill. I'm not familiar with ConditionalWeakTable but if that makes it easy to be wrapped that would be preferred of course. – Dirk Boer Nov 5 '19 at 10:42