ASP.Net Cache and lock for an high traffic Website

Question

This is the scenario:

Thread A ask for All Books in French
Thread B ask for All Books in Arabic

if Thread A is in the lock area, Thread B will be waiting outside the lock area for Thread A to finish.

Because there is only one "place" in the code where Thread B can get his Data.

I want to make a lock based on the Key in the cache which mean like this:

if(Cache["BooksInFrench"] == null)
{
    lock("BooksInFrench")
    {
        if(Cache["BooksInFrench"] == null)
        {
        object d = GetFromDB();
        cache["BooksInFrench"] = d;
        }
}

the same for BooksInArabic etc.... so this way if i have 200 connections to my site and 10 of them ask for BooksInFrench the rest of the requests(BooksInArabic, BooksInHebrew.......) can run and get thier Data beacuse their request is not in the lock area.

my current code looks like this: http://pastebin.com/LFhxgHDM

i think the double locking is not very good... do you see any other solution/improvement?

AFAIK, locks are not meant for readonly objects that are not frequently changing i-e. in your case book in arabic might not be changing frequently, so my point is why are you using locks to read a data that is not changing in real time?? — Furqan Hameedi, Aug 09 '12 at 09:35
Even though it does not change frequently, it might change at the very wrong moment. Whatever *can* go wrong *will* go wrong. — Matthias Meid, Aug 09 '12 at 09:42
@Furqan: because (presumably) fetching the data (`GetFromDB()`) is a relatively costly (time-wise) operation, and that is why it is being cached. — Joel Purra, Aug 15 '12 at 15:32

score 0 · Answer 1 · edited May 23 '17 at 11:56

It might just be in your example pseudocode, but locking on objects is recommended over strings (because of lower level optimizations like immutable shared strings, canonical versions etctera - see Locking by string. Is this safe/sane?).

For a single, known lock:

// In your class
private static readonly object BooksInFrenchLock= new object();
private const string BookInFrenchCacheKey = "BooksInFrench";

// In your cache block
if(Cache[BookInFrenchCacheKey] == null)
{
    lock(BooksInFrenchLock)
    {
        if(Cache[BookInFrenchCacheKey] == null)
        {
            object d = GetFromDB();
            Cache[BookInFrenchCacheKey] = d;
        }
}

In order to lock multiple objects from the database, you could use a ConcurrentDictionary instead of a hashtable (as seen in your pastebin code) - it will save you from one additional "level" of locking. The magic happens in BookLocks.GetOrAdd(bookLanguage, s => new object()).

// In your class
private static readonly ConcurrentDictionary<string, object> BookLocks =
    new ConcurrentDictionary<string, object>();

// In your cache block
string bookLanguage = formatAndCleanBookLanguageArgument();

if(Cache[bookLanguage] == null)
{
    object lockObject = BookLocks.GetOrAdd(bookLanguage, s => new object());

    lock(lockObject)
    {
        if(Cache[bookLanguage] == null)
        {
            object d = GetFromDB();
            Cache[bookLanguage] = d;
        }
}

If you use this with dynamically generated keys, the ConcurrentDictionary will grow and grow over time as locks are never cleaned up, although this might not be a problem. Two solutions might be to 1. remove the lock object in the cacheexpirycallback (but this creates some tricky race conditions) or to 2. use ConditionalWeakTable instead of the dictionary (but this means using a string key with the CWT, which seems to be frowned upon..) — mcintyre321, Aug 18 '14 at 10:17

ASP.Net Cache and lock for an high traffic Website

1 Answers1