【C#】参照型のデータをMemoryCacheで扱ったら事故った話

はじめに

データベースから頻繁に取得するマスターデータを、MemoryCacheでキャッシュしていました。
ライブラリの中で行われていたのでstaticなことをあまり意識せず使用していたのですが、事故ったので書き記します。

結論だけ読みたい方はこちら

問題のあったコード

実際はもっと入り組んでいたんですが、超簡略化すると以下のような感じです。

キャッシュを操作する関数があります

public static class Cache
{
    private static readonly MemoryCache MemoryCache = new MemoryCache(new MemoryCacheOptions());
    
    public static T Get<T>(string key)
    {
        return MemoryCache.Get<T>(key);
    }

    public static void Set(string key, object value)
    {
        MemoryCache.Set(key, value);
    }
}

適当なListをキャッシュしてあげて先頭の要素を削除した後、forで出力してあげます

[Fact]
public void TestSimple()
{
    var key = "key";

    var hogeList = new List<Hoge>()
    {
        new() { Id = "1", Name = "hoge" },
        new() { Id = "2", Name = "fuga" }
    };

    // listをキャッシュにセット
    Cache.Set(key, hogeList);
        
    RemoveFirstKey(key);
}

private void RemoveFirstKey(string key)
{
    // キャッシュからlistを取得して、最初の要素を削除する。
    var hogeList = Cache.Get<List<Hoge>>(key);
    var obj = hogeList.First();
    list.Remove(obj);
    foreach (var hoge in hogeList)
    {
        Console.WriteLine(JsonSerializer.Serialize(hoge));
    }
}

出力

{"Id":"2","Name":"fuga"}

普通に実行できました。
はて？何が問題でしょうか。
では実際にAPIで叩かれることを想定して、並列に実行してみましょう。

Taskで分割して並列実行する

[Fact]
public async void TestCacheAsync()
{
    var key = "key";

    var hogeList = new List<Hoge>()
    {
        new() { Id = "1", Name = "hoge" },
        new() { Id = "2", Name = "hoge" },
        new() { Id = "3", Name = "hoge" },
        new() { Id = "4", Name = "hoge" },
    };

    // listをキャッシュにセットする
    Cache.Set(key, hogeList);

    // キャッシュを取得して先頭を削除する処理を並列実行する
    var taskList = new List<Task>()
    {
        Task.Run(() => RemoveFirstKey(key)),
        Task.Run(() => RemoveFirstKey(key)),
        Task.Run(() => RemoveFirstKey(key)),
    };
    await Task.WhenAll(taskList);
}

出力

System.InvalidOperationException
Collection was modified; enumeration operation may not execute.

例外になりました。
別スレッドで同じ配列を操作している旨の例外っぽいです。
ですがキャッシュから一度取得してから処理を加えているはずです。
解説します。

参照型データのキャッシュは書き換え可能

以下のコードを見てみましょう。

[Fact]
public void TestQiita()
{
    var key = "key";

    var hoge = new Hoge()
    {
        Id = "1", Name = "hoge"
    };

    // キャッシュをセット
    Cache.Set(key, hoge);

    // キャッシュを取得
    var task1 = Cache.Get<Hoge>(key);

    Console.WriteLine(JsonSerializer.Serialize(task1));
    
    task1.Name = "fuga";

    // キャッシュを取得
    var task2 = Cache.Get<Hoge>(key);
    
    Console.WriteLine(JsonSerializer.Serialize(task2));
}

出力

{"Id":"1","Name":"hoge"}
{"Id":"1","Name":"fuga"}

task1変数はキャッシュから取得したオブジェクトのはずです。
それを書き換えているだけ（Setしていない）なのに、task2で再度Getした値が書き換わっています。
このことから参照型をキャッシュした場合、MemoryCacheは参照値をキャッシュしていて、取得できるデータもそのデータの参照値であり書き換え可能ということがわかります。

先ほどの例外は別スレッド間で同じ参照先を持つListを操作してしまったから発生したと推察できます。

対策

MemoryCacheで取得したデータは中身の操作をしない

これが一番ベターだと思います。
暴論な気がしますが、もっと噛み砕くと実質以下のようにしているのと変わらないと思います。

public static class ListCache
{
    private static readonly List<string> List = new List<string>();

    public static List<string> Get()
    {
        return List;
    }

    public static void Set(string value)
    {
        List.Add(value);
    }
}

このように書くと一目でやばいコードだとわかりますね。
このListを操作しようとは思いません。

書き換えが必要な場合は新しいオブジェクトを作る

今回の場合ですと以下のように新しくnewしてあげるだけで解決します。

private void RemoveFirstKey(string key)
{
    var hogeList = Cache.Get<List<Hoge>>(key);
    // 新しくインスタンスを作ってあげる
    var tmpList = new List<Hoge>(hogeList);
    var obj = tmpList.First();
    tmpList.Remove(obj);
    foreach (var hoge in tmpList)
    {
        Console.WriteLine(JsonSerializer.Serialize(hoge));
    }
}

感想

redisやmemcachedのような感覚で使っていたところに事故要素がありました。
C#のstaticとMemoryCacheには気をつけましょう。