patterncsharpMinor
Using different instances of an object to lock threads
Viewed 0 times
instancesthreadsdifferentusingobjectlock
Problem
I'd like to use lock objects that are specific to the person I'm updating. In other words, if thread A is updating Person 1, thread B is blocked from also updating Person 1, but thread C is not blocked from updating Person 2.
99% of the time, I don't really need the locks since I'm working with different Person.Id values. In the occasional situations when I am working with the same Person.Id value, I want to lock around some read/write code.
The following code is working as I expect. I'm looking for any "gotcha's" I may have missed, or better ways of accomplishing the same thing. I decided to use the .NET Cache to store the lock objects so I don't need to worry about cleaning them up later (the lock objects are removed from cache when they haven't been used for a certain TimeSpan).
This code provides the object to lock on
This is how I'm using the code
99% of the time, I don't really need the locks since I'm working with different Person.Id values. In the occasional situations when I am working with the same Person.Id value, I want to lock around some read/write code.
The following code is working as I expect. I'm looking for any "gotcha's" I may have missed, or better ways of accomplishing the same thing. I decided to use the .NET Cache to store the lock objects so I don't need to worry about cleaning them up later (the lock objects are removed from cache when they haven't been used for a certain TimeSpan).
This code provides the object to lock on
public static class PersonLocks
{
private static readonly object CacheLock = new object();
private const string KeyPrefix = "LockForPersonID:";
public static object GetPersonLock(long personId)
{
lock (CacheLock)
{
string key = BuildCacheKey(personId);
object cachedItem = HttpRuntime.Cache[key];
if (cachedItem == null)
{
cachedItem = new object();
HttpRuntime.Cache.Insert(key, cachedItem, null, Cache.NoAbsoluteExpiration, new TimeSpan(0, 5, 0));
}
return cachedItem;
}
}
private static string BuildCacheKey(long personId)
{
return KeyPrefix + personId.ToString();
}
}This is how I'm using the code
object padlock = PersonLocks.GetPersonLock(person.Id);
lock (padlock)
{
//do read
//do some data mapping
//do write
}Solution
Your code would do just fine, in terms of functionality.
However, note that such design holds as long as everybody knows that they should acquire the lock before performing any process on the person object.
Perhaps you should consider re-design your data objects (such as the person class) so that there's an explicit method for any update process. Perhaps something like
This way the locking is made more explicit.
Just a suggestion, though...
However, note that such design holds as long as everybody knows that they should acquire the lock before performing any process on the person object.
Perhaps you should consider re-design your data objects (such as the person class) so that there's an explicit method for any update process. Perhaps something like
BeginEdit and EndEdit, or, even better, a method that returns an IDisposable object that acquires the lock and releases it in its Dispose implementation. Example:public class Person {
public IDisposable AcquireLock() {
// ...
}
// ...
}
// usage:
var aPerson = GetPerson(1234);
using (aPerson.AcquireLock()) {
aPerson.LastName = "abcd";
SavePerson(aPerson);
// ...
}This way the locking is made more explicit.
Just a suggestion, though...
Code Snippets
public class Person {
public IDisposable AcquireLock() {
// ...
}
// ...
}
// usage:
var aPerson = GetPerson(1234);
using (aPerson.AcquireLock()) {
aPerson.LastName = "abcd";
SavePerson(aPerson);
// ...
}Context
StackExchange Code Review Q#14519, answer score: 5
Revisions (0)
No revisions yet.