CODEWITHSUNDEEP
c#locking
Todd Menier
Todd Menier

Reputation: 39329

How do I lock when the ideal scope of the lock object is known only at runtime?

The requirement is fairly straightforward: I want to make sure multiple threads aren't modifying an object at the same time. The tricky part is that the object is coming from a factory whose implementation is unknown until runtime. It may return a singleton, may create a new instance each time, or may have a pool of shared instances.

var thing = factory.Get(...);
lock (???) {
    // modify thing
}

I understand it's not safe to lock on a publicly visible object that other code could potentially lock on, thus creating a possibility for deadlocks. In other words, I shouldn't lock (thing). But ideally I want to lock on something with the same, known-only-at-runtime scope of thing.

One potential solution I came up with is to use a ConcurrentDictionary of objects keyed by thing's hash code:

private static ConcurrentDictionary<int, object> _thingLocks =
    new ConcurrentDictionary<int, object>();

...

var thing = factory.Get(...);
var thingLock = _thingLocks.GetOrAdd(thing.GetHashCode(), new object())

lock (thingLock) {
    // modify thing
}

Intuitively I think this should work, because a) the locks themselves are private so nothing external could also be locking on them, and b) lock instances are highly likely * to be 1-to-1 with thing instances. But since this sort of code is very difficult to test, I wanted to ask: is this a correct and appropriate solution? Is there a better/preferred method of locking on a scope that is known only at runtime?

* - As noted in the comments, that's not guaranteed, but in the unlikely event of a collision it just means 2 things can't be modified at the same time, which is less than ideal but perfectly safe.

Upvotes: 1

Views: 338

Answers (1)

Mike Zboray
Mike Zboray

Reputation: 40828

So you've got a situation where you want some additional data (a lock object) for each instance of class but you don't have control over the implementation or their lifetime. If you could change the implementation you could easily add the lock object inside the class to do whatever it is you want. This suggests a lookup based on the identity of the object, like Dictionary (or related classes like ConcurrentDictionary). This has the problem that you then have to manage the clean up of the dictionary because putting objects as keys creates references to them inside the dictionary. We want something to cleanup the values when the keys are no longer referenced. Enter ConditionalWeakTable. It works kind of like a dictionary lookup on the identity of the key, but keeps a weak reference to it. Note that it does not use the virtual GetHashCode or Equals in its implementation. It always does reference comparisons. Also ConditionalWeakTable is already designed to be thread-safe so it pretty easy to use in place of ConcurrentDictionary:

class Thing
{
    public int Value { get; set; }
}

abstract class ThingFactory
{
    public abstract Thing GetOrCreate(string key);
}

class ThingManager
{
    private readonly ConditionalWeakTable<Thing, object> _locks;
    private readonly ThingFactory _factory;

    public ThingManager(ThingFactory factory)
    {
        _factory = factory;
        _locks = new ConditionalWeakTable<Thing, object>();
    }

    public int Increment(string key,  int incr)
    {
        var thing = _factory.GetOrCreate(key);
        var thingLock = _locks.GetOrCreateValue(thing);

        lock (thingLock)
        {
            int newValue = thing.Value + incr;
            thing.Value = newValue;
            return newValue;
        }
    }
}

Upvotes: 2

Related Questions