15

I'm currently trying out the new MemoryCache in .Net 4 to cache a few bits of data in one of our apps. The trouble I have is the objects are updated and the cache appears to be persisting the changes e.g.

public IEnumerable<SomeObject> GetFromDatabase(){
    const string _cacheKeyGetDisplayTree = "SomeKey"; 
    ObjectCache _cache = MemoryCache.Default;
    var objectInCache = _cache.Get(_cacheKeyGetDisplayTree) as IEnumerable<SomeObject>;
    if (objectInCache != null)
        return objectInCache.ToList();

    // Do something to get the items
    _cache.Add(_cacheKeyGetDisplayTree, categories, new DateTimeOffset(DateTime.UtcNow.AddHours(1)));

    return categories.ToList();
}

public IEnumerable<SomeObject> GetWithIndentation(){
    var categories = GetFromDatabase();

    foreach (var c in categories)
    {
        c.Name = "-" + c.Name;
    }

    return categories;
}

If I were calling GetWithIndentation() first and then later calling GetFromDatabase() I would expect it to return the original list of SomeObject but instead it returns the modified items (with "-" prefixed on the name).

I thought ToList() destroyed the reference but it still seems to persist the changes. I'm sure it's obvious but can anyone spot where I'm going wrong?

Ian R. O'Brien
  • 6,682
  • 9
  • 45
  • 73
Tim
  • 5,443
  • 2
  • 22
  • 26
  • 4
    You are making a copy of the collection, but not the objects inside it. – Ilia G Dec 18 '12 at 14:09
  • Ah good spot thanks. Now I'm wondering the best way around this, would it be to do a deep copy or is there a way of telling MemoryCache to ignore subsequent changes? – Tim Dec 18 '12 at 14:17
  • 1
    Either modify it before caching or never. Besides this is such a trivial change, it should probably be done in your presentation layer. – Ilia G Dec 18 '12 at 14:39
  • I've simplified the code for explanatory reasons, there's a lot more going on within `GetWithIndentation()` and it's accessed from multiple places. I can cache the output of `GetWithIndentation()` I guess but `GetFromDatabase()` is accessed from other methods hence why I felt it was a good place to cache the db data. – Tim Dec 18 '12 at 16:21
  • Well, in that case you either have to maintain 2 caches or `GetWithIndentation` needs to clone category objects and apply changes there. – Ilia G Dec 18 '12 at 16:43

5 Answers5

7

I created a ReadonlyMemoryCache class to solve this problem. It inherits from the .NET 4.0 MemoryCache, but objects are stored readonly (by-value) and cannot be modified. I deep copy the objects before storing using binary serialization.

using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.IO;
using System.Runtime.Caching;
using System.Runtime.Serialization.Formatters.Binary;
using System.Threading.Tasks;


namespace ReadOnlyCache
{
    class Program
    {

        static void Main()
        {
            Start();
            Console.ReadLine();
        }

        private static async void Start() {
            while (true)
            {
                TestMemoryCache();
                await Task.Delay(TimeSpan.FromSeconds(1));
            }
        }

        private static void TestMemoryCache() {
            List<Item> items = null;
            string cacheIdentifier = "items";

            var cache = ReadonlyMemoryCache.Default;

            //change to MemoryCache to understand the problem
            //var cache = MemoryCache.Default;

            if (cache.Contains(cacheIdentifier))
            {
                items = cache.Get(cacheIdentifier) as List<Item>;
                Console.WriteLine("Got {0} items from cache: {1}", items.Count, string.Join(", ", items));

                //modify after getting from cache, cached items will remain unchanged
                items[0].Value = DateTime.Now.Millisecond.ToString();

            }
            if (items == null)
            {
                items = new List<Item>() { new Item() { Value = "Steve" }, new Item() { Value = "Lisa" }, new Item() { Value = "Bob" } };
                Console.WriteLine("Reading {0} items from disk and caching", items.Count);

                //cache for x seconds
                var policy = new CacheItemPolicy() { AbsoluteExpiration = new DateTimeOffset(DateTime.Now.AddSeconds(5)) };
                cache.Add(cacheIdentifier, items, policy);

                //modify after writing to cache, cached items will remain unchanged
                items[1].Value = DateTime.Now.Millisecond.ToString();
            }
        }
    }

    //cached items must be serializable

    [Serializable]
    class Item {
        public string Value { get; set; }
        public override string ToString() { return Value; }
    }

    /// <summary>
    /// Readonly version of MemoryCache. Objects will always be returned in-value, via a deep copy.
    /// Objects requrements: [Serializable] and sometimes have a deserialization constructor (see http://stackoverflow.com/a/5017346/2440) 
    /// </summary>
    public class ReadonlyMemoryCache : MemoryCache
    {

        public ReadonlyMemoryCache(string name, NameValueCollection config = null) : base(name, config) {
        }

        private static ReadonlyMemoryCache def = new ReadonlyMemoryCache("readonlydefault");

        public new static ReadonlyMemoryCache Default {
            get
            {
                if (def == null)
                    def = new ReadonlyMemoryCache("readonlydefault");
                return def;
            }
        }

        //we must run deepcopy when adding, otherwise items can be changed after the add() but before the get()

        public new bool Add(CacheItem item, CacheItemPolicy policy)
        {
            return base.Add(item.DeepCopy(), policy);
        }

        public new object AddOrGetExisting(string key, object value, DateTimeOffset absoluteExpiration, string regionName = null)
        {
            return base.AddOrGetExisting(key, value.DeepCopy(), absoluteExpiration, regionName);
        }

        public new CacheItem AddOrGetExisting(CacheItem item, CacheItemPolicy policy)
        {
            return base.AddOrGetExisting(item.DeepCopy(), policy);
        }

        public new object AddOrGetExisting(string key, object value, CacheItemPolicy policy, string regionName = null)
        {
            return base.AddOrGetExisting(key, value.DeepCopy(), policy, regionName);
        }

        //methods from ObjectCache

        public new bool Add(string key, object value, DateTimeOffset absoluteExpiration, string regionName = null)
        {
            return base.Add(key, value.DeepCopy(), absoluteExpiration, regionName);
        }

        public new bool Add(string key, object value, CacheItemPolicy policy, string regionName = null)
        {
            return base.Add(key, value.DeepCopy(), policy, regionName);
        }

        //for unknown reasons, we also need deepcopy when GETTING values, even though we run deepcopy on all (??) set methods.

        public new object Get(string key, string regionName = null)
        {
            var item = base.Get(key, regionName);
            return item.DeepCopy();
        }

        public new CacheItem GetCacheItem(string key, string regionName = null)
        {
            var item = base.GetCacheItem(key, regionName);
            return item.DeepCopy();
        }

    }


    public static class DeepCopyExtentionMethods
    {
        /// <summary>
        /// Creates a deep copy of an object. Must be [Serializable] and sometimes have a deserialization constructor (see http://stackoverflow.com/a/5017346/2440) 
        /// </summary>
        public static T DeepCopy<T>(this T obj)
        {
            using (var ms = new MemoryStream())
            {
                var formatter = new BinaryFormatter();
                formatter.Serialize(ms, obj);
                ms.Position = 0;

                return (T)formatter.Deserialize(ms);
            }
        }
    }



}
Sire
  • 4,086
  • 4
  • 39
  • 74
  • This one looks like a winner, the DeepCopyExtensionMethods class is what solves the problem. – user917170 Feb 27 '15 at 03:41
  • @GrantWinney If you remove the deepcopy from Get() you will see it doesn't work. I agree it shouldn't be necessary. – Sire Sep 25 '15 at 10:44
2

In memory cached objects are stored within the same process space as the cache client process. When a cache client requests a cached object, the client receives a reference to the locally cached object rather than a copy.

The only way to get a clean copy of the object is to implement a custom clone mechanism (ICloneable, Serialization, Automapping, ...). With that copy you will be able to alter the new object without altering the parent object.

Depending on your use case, it is generally not recommanded to update an object in the cache.

Cybermaxs
  • 24,378
  • 8
  • 83
  • 112
  • "Depending on your use case, it is generally not recommanded to update an object in the cache." Would you mind elaborating on why this is? As a trivial example, say you had a list of users stored in cache. Users would infrequently change, but they would change at some point (e.g. update first/last name, address, etc.) - would this make a good candidate for storing a collection of users in cache and updating them when need be? – Mark Erasmus Sep 15 '15 at 02:42
1

You can do it easier if you deserialize and serialize again and get your cache object "By Value".

You can do it like this with Newtonsoft lib (just get it from NuGet)

var cacheObj = HttpRuntime.Cache.Get(CACHEKEY);
var json = JsonConvert.SerializeObject(cacheObj);
var byValueObj = JsonConvert.DeserializeObject<List<string>>(json);
return byValueObj;
Kabindas
  • 802
  • 9
  • 6
0

Why not just store as json or a string? These are not passed by reference and when you get out of the cache you will get a new copy :) I am here to be challenged as thats what I am doing atm!

0

Serialization/Deserialization will solve the problem but at the same time it defeats the porpose of having objects in memory. The role of cache is to provide fast access to the stored object and we are adding the deserialization overhead here. Since deserialization is required I would suggest cache as service , something like redis cache, it will be centralized so you don't have to have copy of memory object per worker process and deserialization is anyways done.

The key thing in this case that you chose a fast serialization/deserialization option.

Gaurav
  • 330
  • 7
  • 21