HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Associating a lock/mutex with the data it protects

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mutexthewithassociatingdataprotectslock

Problem

I've recently come across some areas of code in our solution that have a few thread-safety problems. Thread-safety is a minefield at best so I thought I'd have an attempt at writing a few little utility classes that would make it easier to write safe multi-threaded code.

The general idea is to link a resource with the lock(s) that protect it. If the resource is accessed outside of all of it's locks then a runtime exception is thrown.

Imagine a developer had written the following (example) caching object but had forgotten to take the _imageLock when clearing the cache.

Using the traditional .net locking mechanism (of just locking on an Object) we might corrupt the Dictionary if two threads call Add() and ClearCache() at the same time.

Using these utility classes, Padlock and Protected, will instead throw an exception in the ClearCache() method because the Protected object is being accessed outside of it's lock. This exception should surface during the developers normal testing procedure, before the code is even checked in.

public class SomeThreadSafeCache
{
    Padlock _imageLock = new Padlock();
    Protected> _imageCache;

    public SomeThreadSafeCache()
    {
        //The variable _imageCache is associate with the _imageLock Padlock
        //i.e. _imageCache is only available when we're locking on _imageLock.
        _imageCache = new Protected>(
            new Dictionary(),
            _imageLock);
    }

    public void AddImage(string key, byte[] data)
    {
        using (_imageLock.Lock())
        {
            _imageCache.Value.Add(key, data);
        }
    }

    public void ClearCache()
    {
        //Throws an exception because the protected
        //resource is accessed outside of it's lock.
        _imageCache.Value.Clear();
    }
}


(I know that .net now comes with Concurrent collections which make my example pointless but the general idea of linking a resource with it's lock is what I'm trying to get at)

The Protected class i

Solution

I confess that I think your approach looks painful. As you can see from the other answers, it's insufficient to protect your data outside the container class. And there's confusion in your example code as to whether Protected is the container or ThreadSafeCache is the container. I think instead that you need to revisit the principle of encapsulation. When you have a collection that requires multi-threaded updates and access, you protect that collection with a wrapper class. The wrapper encapsulates the underlying collection in its entirety. Think "private". If you don't want to expose the lock, then you don't expose an iterator/enumerator. Rather, you do something similar to the List's ForEach method:

public void ForEach(Action t){
   lock(_items) foreach(var item in _items) t.Invoke(item);
}

Code Snippets

public void ForEach(Action<T> t){
   lock(_items) foreach(var item in _items) t.Invoke(item);
}

Context

StackExchange Code Review Q#3329, answer score: 3

Revisions (0)

No revisions yet.