Locking by string. Is this safe/sane?
c#, locking, multithreading
Solution
Another option is to get the HashCode of each URL, then divide it by a prime number and use it as an index into an array of locks. This will limit the number of locks you need while letting you control the probability of a “false locking” by choose the number of locks to use.
However the above is only worthwhile if it is too costly just have one lock per active url.
Problem
I need to lock a section of code by string. Of course the following code is hideously unsafe: ``` lock("http://someurl") { //bla } ``` So I've been cooking up an alternative. I'm not normally one to post large bodies of code here, but when it comes to concurrent programming, I'm a little apprehensive about making my own synchronization scheme, so I'm submitting my code to ask if it's sane to do it in this way or whether there's a more straightforward approach. ``` public class StringLock { private readonly Dictionary<string, LockObject> keyLocks = new Dictionary<string, LockObject>(); private readonly object keyLocksLock = new object(); public void LockOperation(string url, Action action) { LockObject obj; lock (keyLocksLock) { if (!keyLocks.TryGetValue(url, out obj)) { keyLocks[url] = obj = new LockObject(); } obj.Withdraw(); } Monitor.Enter(obj); try { action(); } finally { lock (keyLocksLock) { if (obj.Return()) { keyLocks.Remove(url); } Monitor.Exit(obj); } } } private class LockObject { private int leaseCount; public void Withdraw() { Interlocked.Increment(ref leaseCount); } public bool Return() { return Interlocked.Decrement(ref leaseCount) == 0; } } } ``` I would use it like this: ``` StringLock.LockOperation("http://someurl",()=>{ //bla }); ``` Good to go, or crash and burn? EDIT For posterity, here's my working code. Thanks for all the suggestions: ``` public class StringLock { private readonly Dictionary<string, LockObject> keyLocks = new Dictionary<string, LockObject>(); private readonly object keyLocksLock = new object(); public IDisposable AcquireLock(string key) { LockObject obj; lock (keyLocksLock) { if (!keyLocks.TryGetValue(key, out obj)) { keyLocks[key] = obj = new LockObject(key); } obj.Withdraw(); } Monitor.Enter(obj); return new DisposableToken(this, obj); } private void ReturnLock(DisposableToken disposableLock) { var obj = disposableLock.LockObject; lock (keyLocksLock) { if (obj.Return()) { keyLocks.Remove(obj.Key); } Monitor.Exit(obj); } } private class DisposableToken : IDisposable { private readonly LockObject lockObject; private readonly StringLock stringLock; private bool disposed; public DisposableToken(StringLock stringLock, LockObject lockObject) { this.stringLock = stringLock; this.lockObject = lockObject; } public LockObject LockObject { get { return lockObject; } } public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } ~DisposableToken() { Dispose(false); } private void Dispose(bool disposing) { if (disposing && !disposed) { stringLock.ReturnLock(this); disposed = true; } } } private class LockObject { private readonly string key; private int leaseCount; public LockObject(string key) { this.key = key; } public string Key { get { return key; } } public void Withdraw() { Interlocked.Increment(ref leaseCount); } public bool Return() { return Interlocked.Decrement(ref leaseCount) == 0; } } } ``` Used as follows: ``` var stringLock=new StringLock(); //... using(stringLock.AcquireLock(someKey)) { //bla } ```