Locking a static field

Question

I'm maintaining an application that consumes a common library that has a static instance of a class(ClassWrapper). This class is a basically a wrapper around the Microsoft patterns and practices's CacheManager (v3.1 of the EL)

The library is hosted in a web application and also in a windows service app, (both are inherently multi threaded) There are a bunch of places in the app that invokes an Add method on this wrapper which in turn calls the Add on the cache manager to add an item to the cache manager.

As I understand, the CacheManager is not thread safe and the CacheWrapper does not perform any locking to ensure thread safety in the call to Add.

I cannot modify the library code directly to add synhronization code and am considering writing a helper method like so and modifying all call sites to use this helper instead of calling the Add on the wrapper directly.

class CacheHelper
{
    private static object _syncLock = new object();

    public static void Add<T>(CacheWrapper wrapper, string key, T value, int expireInMins)
    {
        lock (_syncLock)
        {
            wrapper.Add(key, value, expireInMins);
        }
    }
}

Do you see any problems with this approach. I'm a bit weary since the CacheWrapper is static and hence inherently so is the _syncLock. I feel a bit uneasy locking on static objects, but I don't have much of a choice since the CacheWrapper is a static instance exposed all throughout the process space of the host (web app and the windows service).

Any advice or vote of confidence would be much appreciated.

Answer 1

I am not sure about CacheManager being not thread-safe. Check this MSDN article - it states clearly:

Every method call made through the CacheManager object is thread safe.

Now, coming to your implementation, I am not certain why you are passing the CacheWrapper instance to your methods. CacheWrapper being static instance you can refer it directly such as

class CacheHelper
{

    private static CacheWrapper GetWrapper()
    {
       return [Lib Namespace].[Class Name].[Field Name referring to CacheWrapper];
    }


    public static void Add<T>(string key, T value, int expireInMins)
    {
        var wrapper = GetWrapper();
        lock (wrapper)
        {
            wrapper.Add(key, value, expireInMins);
        }
    }

    ...

Again, GetWrapper is a factory method and implementation can change - it can use a static delegate to get CacheWrapper instance OR use dependency injection to get the reference to CacheWrapper.

Another advantage here is that if there are multiple instances for CacheWrapper then you would take lock only on one that you are using currently.