Locking in a factory method

Question

I am interfacing with a back-end system, where I must never ever have more than one open connection to a given object (identified by it's numeric ID), but different consumers may be opening and closing them independently of one another.

Roughly, I have a factory class fragment like this:

private Dictionary<ulong, IFoo> _openItems = new Dictionary<ulong, IFoo>();
private object _locker = new object();

public IFoo Open(ulong id)
{
    lock (_locker)
    {
        if (!_openItems.ContainsKey(id))
        {
            _openItems[id] = _nativeResource.Open(id);
        }

        _openItems[id].RefCount++;

        return _openItems[id];
    }
}

public void Close(ulong id)
{
    lock (_locker)
    {
        if (_openItems.ContainsKey(id))
        {
            _openItems[id].RefCount--;
            if (_openItems[id].RefCount == 0)
            {
                _nativeResource.Close(id);
                _openItems.Remove(id);
            }
        }
    }
}

Now, here is the problem. In my case, _nativeResource.Open is very slow. The locking in here is rather naive and can be very slow when there are a lot of different concurrent .Open calls, even though they are (most likely) referring to different ids and don't overlap, especially if they are not in the _openItems cache.

How do I structure the locking so that I am only preventing concurrent access to a specific ID and not to all callers?

Answer 1

What you may want to look into is a striped locking strategy. The idea is that you share N locks for M items (possible ID's in your case), and choose a lock such that for any ID the lock chosen is always the same one. The classic way of choosing locks for this technique is modulo division- simply divide M by N, take the remainder, and use the lock with that index:

// Assuming the allLocks class member is defined as follows:
private static AutoResetEvent[] allLocks = new AutoResetEvent[10];


// And initialized thus (in a static constructor):
for (int i = 0; i < 10; i++) {
    allLocks[i] = new AutoResetEvent(true);
}


// Your method becomes
var lockIndex = id % allLocks.Length;
var lockToUse = allLocks[lockIndex];

// Wait for the lock to become free
lockToUse.WaitOne();
try {
    // At this point we have taken the lock

    // Do the work
} finally {
    lockToUse.Set();
}