C# Static Property Locking

Question

Just looking for a code review of this code. ASP.net Cache is not an option. The static list will be accessed a lot on a website that gets well over 10K page views per day and concurrent read attempts is likely. On app restart when the list is rebuilt I was wondering if there are any issues I may be overlooking? Is locking on the list being instantiated best practice?

public class MyClass
{
        private static List<Entry> _listCache = null; 
        protected static List<Entry> ListCache
        {
            get
            {

                if (_listCache == null)
                {
                    _listCache = new List<Entry>();
                    lock (_listCache)
                    {
                        //Add items to the list _listCache from XML file
                    }
                }
                return _listCache;
            }
        }
        //....Other methods that work with the list
}

Answer 1

10k views - that's one every 8 seconds... not sure you need to worry too much... ;-p

But re the code - that is overcomplicating things, and you could still end up initializing it twice. I'd just use a static constructor to do this; it'll be more robust. If you must have full isolated lazy loading (even with other static methods on the type), there is a trick with an inner class to achieve the same:

public class MyClass
{
    static class InnerCache {
        internal static readonly IList<Entry> _listCache;
        static InnerCache() {
            List<Entry> tmp  = new List<Entry>();
            //Add items to the list _listCache from XML file
            _listCache = new ReadOnlyCollection<Entry>(tmp);
        }
    }
    protected static IList<Entry> ListCache {
        get {return InnerCache._listCache;}
    }
}

I would also be concerned about the chance of somebody mutating the list - might want to use a readonly list!

Answer 2

There's not really a reason this wouldn't work for you. However, if you want to do it the way your sample code is, you want to lock before you check to see if _listCache is null. So you would need a separate monitor to lock on. Something like this:

public class MyClass
{
        private static object _listCacheMonitor = new object();
        private static List<Entry> _listCache = null; 
        protected static List<Entry> ListCache
        {
            get
            {
                lock (_listCacheMonitor) {    
                    if (_listCache == null)
                    {
                        _listCache = new List<Entry>();
                        //Add items to the list _listCache from XML file
                    }
                }
                return _listCache;
            }
        }
        //....Other methods that work with the list
}