0

When inserting and retrieving items into cache. Is it best practice to perform a lock on the cache. I've following the guidelines from the following post.

What is the best way to lock cache in asp.net?

But i'm keen to get some context on whether i need to lock or not. This is the only location i use this cache.

private static object _tcsLock = new object();

public TcsService()
{

}


public LiveScoreTcs GetScoreCard()
{

    // try to pull from cache here
    var scorecard = HttpContext.Current.Cache[Global.Caching.SecondXILiveScoreKey];
    if (scorecard != null)
    {
        return (LiveScoreTcs)scorecard;
    }

    lock (_tcsLock)
    {
        // cache was empty before we got the lock, check again inside the lock
        scorecard = HttpContext.Current.Cache[Global.Caching.SecondXILiveScoreKey];
        // cache is still empty, so retreive the value here
        if (scorecard == null)
        {
            scorecard = BuildSecondXiLiveScorecard();
            // store the value in the cache here
            if (scorecard != null)
            {
                HttpContext.Current.Cache.Insert(Global.Caching.SecondXILiveScoreKey, scorecard,
                null,
                Global.Caching.AbsoluteExpiration.Tcs,
                Cache.NoSlidingExpiration,
                CacheItemPriority.Default,
                null);
            }
        }

    }

    // return the cached value here
    return (LiveScoreTcs)scorecard;
}
Community
  • 1
  • 1
frosty
  • 5,330
  • 18
  • 85
  • 122
  • Where is `_tcsLock` declared and/or initialized? – Afzaal Ahmad Zeeshan Jun 09 '15 at 16:16
  • just updated.. thanks – frosty Jun 09 '15 at 16:26
  • 1
    Please note that `Cache` will have all the normal issues of any shared state in a multi-threaded environment. On top of that, there may be some issues which are specific to `Cache`, but the answer to "what are the use cases" is "all the normal use cases, plus maybe some `Cache`-specific use cases". – John Saunders Jun 09 '15 at 16:32

2 Answers2

1

Locking is not about where in the code you use it but if more than one thread can execute that code at any one time. If there is a chance of a race condition you need to lock, even if apparently in development and your specific setup no errors emerge. This is so because multi-threading is impacted by so many factors that you cannot predict the behavior.

So if this is a multi-threaded application (web server) you need to lock.

Reducing the amount of code that is locked is also a good advice, as collisions put threads into the waiting state you want to make them wait as less as possible.

If you think you have many read accesses and few write accesses you may want to look into more complex mechanisms like Semaphore and adopt a Multiple Reader Single Writer pattern.

Really, it all depends on what you do in the rest of the application (threads or parallel web requests).

pid
  • 11,472
  • 6
  • 34
  • 63
  • This isn't addressing the specific situation in the question at all. – Servy Jun 09 '15 at 16:25
  • @Servy: it depends. If the question is the code, then it addresses it. If the question is the text in the post, then maybe not. The OP thinks he needs to "lock the cache" when he needs to lock the objects he's putting into the cache. – John Saunders Jun 09 '15 at 16:28
  • @JohnSaunders This answer is just speaking generally about locking, it's in no way specific to the situation that the OP is actually asking about. I mean, he first starts out saying that it's only relevant if there are multiple threads, but as we know it's an ASP app we *know* the code can be run by lots of threads at the same time, and we know that the lock doesn't need to follow a reader/writer pattern, and it does nothing to explain why the lock is there, and what the consequences of it not being there are. – Servy Jun 09 '15 at 16:31
  • @Servy: see my comment to the question. I hope the OP will use that to clarify his question. – John Saunders Jun 09 '15 at 16:32
  • 1
    @Servy I agree. Your description of my answer is correct. But when he asks for *"some context on whether i need to lock or not"* what I see is that he copied code without understanding the greater picture. He asks about *WHY* not *HOW* (which he already solved). I gave intentionally a general picture about locking. – pid Jun 09 '15 at 18:04
  • @pid You gave information about locking, irregardless of the context he *specifically* asked about. He wants to know if he needs the lock, and you've not given him an answer to that question. He has no way of knowing, given your answer, whether he needs the lock, or what it may or may not be accomplishing. You're *not* explaining the "why" that he's asking. – Servy Jun 09 '15 at 18:07
1

If you don't use the lock as you've done than you have the potential to end up re-creating the object you're going to cache multiple times. In some circumstances that's a major problem, particularly if doing the work causes side effects which are important to not perform multiple times, and in some cases doing the work multiple times wastes a bit of time, and is a bit less performant, which may or may not be something you need to avoid.

So whether or not it's okay to ensure BuildSecondXiLiveScorecard is never executed more than once until it expires from the cache is something we can't say, but that's the only potential concern with not having the lock.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • Another case where it would be a problem is if there's really meant to be only a single instance of `scorecard` that all threads use. Even if it were cheap to create them, you may not want thread 1 to use one instance and thread 2 to use another. The code may count on there being only a single instance. – John Saunders Jun 09 '15 at 16:30
  • @JohnSaunders Yeah, but as the cache can expire and require re-creating of the object, you wouldn't actually be able to rely on that in either case. – Servy Jun 09 '15 at 16:32
  • You would - there would be either zero or one instance if you lock. There would be no chance of there being two instances. – John Saunders Jun 09 '15 at 16:33
  • @JohnSaunders Until it expires from the cache, at which point there can be an object still using the old object that it fetched from the cache earlier, and another object that creates a new instance because the cache was empty. It would only be a singleton if the cache never expired the object. – Servy Jun 09 '15 at 16:36
  • Yeah, good point. There might need to be cleanup of the object when it expires from cache. For instance, if the object in Cache were holding onto database connections or something else that implements `IDisposable`. – John Saunders Jun 09 '15 at 17:46
  • @JohnSaunders That's assuming that it needs to be a singleton in the first place. If it does, it probably just shouldn't be cached in the first place. It probably isn't, and likely shouldn't be, an object that requires only having one object instance in existence at any given point in time. – Servy Jun 09 '15 at 17:48