Caching and threadsafety

Question

I am caching data in an ASP.NET website through the System.Web.Caching.Cache-Class, because retrieving the data is very costly and it changes only once in a while, when our content people change data in the backend.

So I create the data in Application_Start and store it in Cache, with an expiration time of 1 day.

When accessing the data (happens on many pages of the website), I have something like this now in a static CachedData class:

public static List<Kategorie> GetKategorieTitelListe(Cache appCache)
{
    // get Data out of Cache
    List<Kategorie> katList = appCache[CachedData.NaviDataKey] as List<Kategorie>;
    // Cache expired, retrieve and store again
    if (katList == null)
    {
            katList = DataTools.BuildKategorienTitelListe();
            appCache.Insert(CachedData.NaviDataKey, katList, null, DateTime.Now.AddDays(1d), Cache.NoSlidingExpiration);
    }
    return katList;
}

The problem I see with this code is that its not threadsafe. If two users open two of these pages at the same time and the cache just ran out, there is a risk the data while be retrieved multiple times.

But if I lock the method body, I will run into performance troubles, because only one user at a time can get the data list.

Is there an easy way to prevent this? What's best practice for a case like this?

Vlad Bezden · Accepted Answer · 2011-07-29T18:00:50.710

4

You are right, your code is not thread safe.

// this must be class level variable!!!
private static readonly object locker = new object();

    public static List<Kategorie> GetKategorieTitelListe(Cache appCache)
    {
        // get Data out of Cache
        List<Kategorie> katList = appCache[CachedData.NaviDataKey] as List<Kategorie>;

        // Cache expired, retrieve and store again
        if (katList == null)
        {
            lock (locker)
            {
                katList = appCache[CachedData.NaviDataKey] as List<Kategorie>;

                if (katlist == null)  // make sure that waiting thread is not executing second time
                {
                    katList = DataTools.BuildKategorienTitelListe();
                    appCache.Insert(CachedData.NaviDataKey, katList, null, DateTime.Now.AddDays(1d), Cache.NoSlidingExpiration);
                }
            }
        }
        return katList;
    }

edited Jul 29 '11 at 18:00

answered Jul 29 '11 at 16:21

Vlad Bezden

83,883
25
248
179

If two thread enter the method at the same time when the data is null, the first one will enter the lock. The second will wait for the lock to "unlock" and then will enter it. `katlist` will still be null in his case, so `DataTools.BuildKetegorienTiteListe` will be called two times. So basically, I think the `lock` statement must englobe all the content of the method, except the `return`. – Johnny5 Jul 29 '11 at 17:18
2

Actually not. In your scenario when 2 thread enter the lock only one will enter the lock and second will wait. So when first thread loads the cache and exit (unlock the lock) second thread will enter and that is why you have second if (katlist == null), so since first thread updated it it will not be null and second thread will not reaload cache. This is the safest way of doing synchronization. This is called double-checked locking. Read more about it at http://en.wikipedia.org/wiki/Double-checked_locking – Vlad Bezden Jul 29 '11 at 17:26
2

In your example from wikipedia, the double check is done on a static bool. In the code you provide, `katList` is just a local variable. If it is null before entering the lock, it will still be null when entering it. – Johnny5 Jul 29 '11 at 17:35
1

Good point, I did not realized that this is local variable, and not class or global variable. +1 to you. I will change my code. – Vlad Bezden Jul 29 '11 at 17:58
Is there a performance issue with this, or after the first user gets the cache, all the following users are released to it. Is there any issue with crashing the site with this setup. – Mike Flynn Apr 24 '21 at 17:36

score 0 · Answer 2 · edited May 23 '17 at 12:32

The MSDN documentation states that the ASP.NET Cache class is thread safe -- meaning that their contents are freely accessible by any thread in the AppDomain (a read/write will be atomic for example).

Just keep in mind that as the size of the cache grows, so does the cost of synchronization. You might want to take a look at this post

By adding a private object to lock on, you should be able to run your method safely so that other threads do not interfere.

private static readonly myLockObject = new object();

public static List<Kategorie> GetKategorieTitelListe(Cache appCache)
{
    // get Data out of Cache
    List<Kategorie> katList = appCache[CachedData.NaviDataKey] as List<Kategorie>;

    lock (myLockObject)
    {
        // Cache expired, retrieve and store again
        if (katList == null)
        {
            katList = DataTools.BuildKategorienTitelListe();
            appCache.Insert(CachedData.NaviDataKey, katList, null, DateTime.Now.AddDays(1d), Cache.NoSlidingExpiration);
        }
        return katList;
    }
}

score 0 · Answer 3 · answered Jul 29 '11 at 17:58

I dont see other solution than locking.

private static readonly object _locker = new object ();

public static List<Kategorie> GetKategorieTitelListe(Cache appCache)
{
    List<Kategorie> katList;

    lock (_locker)
    {
        // get Data out of Cache
        katList = appCache[CachedData.NaviDataKey] as List<Kategorie>;
        // Cache expired, retrieve and store again
        if (katList == null)
        {
                katList = DataTools.BuildKategorienTitelListe();
                appCache.Insert(CachedData.NaviDataKey, katList, null, DateTime.Now.AddDays(1d), Cache.NoSlidingExpiration);
        }
    }
    return katList;
}

Once the data is in the cache, concurrent threads will only wait the time of getting the data out, i.e. this line of code:

katList = appCache[CachedData.NaviDataKey] as List<Kategorie>;

So the performance cost will not be dramatic.

Caching and threadsafety

3 Answers3