Are there any nasty side affects if i lock the HttpContext.Current.Cache.Insert method

Question

Apart from blocking other threads reading from the cache what other problems should I be thinking about when locking the cache insert method for a public facing website.

The actual data retrieval and insert into the cache should take no more than 1 second, which we can live with. More importantly i don't want multiple thread potentially all hitting the Insert method at the same time.

The sample code looks something like:

public static readonly object _syncRoot = new object();

if (HttpContext.Current.Cache["key"] == null)
{
  lock (_syncRoot)
  {
    HttpContext.Current.Cache.Insert("key", "DATA", null, DateTime.Now.AddMinutes(5), Cache.NoSlidingExpiration, CacheItemPriority.Normal, null);
  }
}

  Response.Write(HttpContext.Current.Cache["key"]);

Answer 1

I expect that you do this to prevent the data retrieval is done more than once, perhaps because the amount of data is high, which might have impact on your server when multiple users trigger that retrieval.

A lock like this just on the Cache.Insert itself is useless, because this method is thread-safe. A lock like this can be useful to prevent double data retrieval, but in that case you should consider using a double checked lock:

var  data = HttpContext.Current.Cache["key"];
if (data == null)
{
  lock (_syncRoot)
  {
    // Here, check again for null after the lock.
    var  data = HttpContext.Current.Cache["key"];
    if (data == null)
    {
        var data = [RETRIEVE DATA]
        HttpContext.Current.Cache.Insert("key", data, null, ...);
  }
}
return data;

But to your main question. Apart from the risk of locking for a too long period of time, causing large delays in your web application, there is nothing to worry about :-). A lock around a Cache.Insert by itself, will do you no harm.