An Interesting Case Of .NET Performance and Caching

Several months ago, Bill Boga and I, realized there was a performance enhancement we could make in our infrastructure. We were utilizing IdentityServer and took notice that many of our APIs were validating many of the same access tokens and storing the results in memory locally. Validating access tokens can be relatively expensive due to additional web requests and CPU intensive hash calculations. If we could cache the validation results, and share the validation results across our system, then we could reduce the work necessary to get a request through our APIs.

We decided to write a RedisTokenCache as a shared cache provider for access tokens, which you can see below. Since Redis deals with strings, we used JSON.NET to serialize to a string and deserialize back to the original type. SHIP IT!

public class RedisTokenCache : ICache
{
    public RedisTokenCache(ConnectionMultiplexer connection)
    {
        database = connection.GetDatabase();
    }

    private readonly IDatabase database;

    public bool Add(string key, object value, DateTimeOffset absoluteExpiration)
    {
        return database.StringSet(key, JsonConvert.SerializeObject(value), (absoluteExpiration - DateTimeOffset.UtcNow));
    }

    public object Get(string key)
    {
        var value = database.StringGet(key);

        return string.IsNullOrEmpty(value)
            ? null
            : JsonConvert.DeserializeObject(value);
    }
}

During this time, we also upgraded our servers, so we naturally got a performance boost, which at the time we incorrectly attributed to our optimization.

Recently, we noticed in Application Insights that our access tokens were validated more than we wanted, and it was causing performance degradation. After our investigation (mostly Bill), we realized something serious. Our cache was not working!

Our investigation led us to several problems with our cache implementation:

1. The Claim class is serializable to JSON, but cannot be deserialized due to a lack of empty public constructor.
2. JSON.NET silently adjusted the IEnumerable<Claim> to a JArray, so we were still getting a cache hit.
3. IdentityServer assumed that any value could be implicitly cast to an IEnumerable<Claim> using the as keyword.

By the time IdentityServer received the validation result, it was null and forced us to validate the token every time.

What’s the fix? A better cache implementation.

public class RedisTokenCache : ICache
{
    public RedisTokenCache(ConnectionMultiplexer connection)
    {
        database = connection.GetDatabase();
        settings = new JsonSerializerSettings {
            TypeNameHandling = TypeNameHandling.All,
            Converters = new List<JsonConverter> {
                new ClaimConverter()
            }
        };
    }

    private readonly IDatabase database;
    private readonly JsonSerializerSettings settings;

    public bool Add(string key, object value, DateTimeOffset absoluteExpiration)
    {
        return database.StringSet(key,
            JsonConvert.SerializeObject(value, settings),
            (absoluteExpiration - DateTimeOffset.UtcNow)
        );
    }

    public object Get(string key)
    {
        var value = database.StringGet(key);

        return string.IsNullOrEmpty(value)
            ? null
            : JsonConvert.DeserializeObject(value, settings);
    }
}

To solve this issue, we enhanced our cache with several improvements:

1. Use TypeNameHandling.All which includes Type information with our JSON. The change allows JSON.NET to make better deserialization decisions.
2. Write a ClaimConverter to address the lack of public constructor on the Claim class.

Conclusion

The bug was evident once we started debugging locally. It was harder to see the issue in Windows Azure because our memory metrics and cache hits around Redis showed the service “worked.” Our problems happened on reads in our app. IdentityServer’s InMemoryValidationResultCache assumed any cache hit was a good hit, which hid the issue instead of failing immediately.

/// <summary>
/// Retrieves a validation result
/// </summary>
/// <param name="token">The token.</param>
/// <returns></returns>
public Task<IEnumerable<Claim>> GetAsync(string token)
{
    var result = _cache.Get(token);

    if (result != null)
    {
        return Task.FromResult(result as IEnumerable<Claim>);
    }

    return Task.FromResult<IEnumerable<Claim>>(null);
}

In our case, the cached result was a JArray, but the code assumes it will always be an IEnumerable<Claim> which will turn our value into a null.

What’s the moral of this story?

1. Trust what you see in your analysis, not what you think should be happening.
2. When caching data, be sure that you can serialize and deserialize the values.
3. Just because you were able to get a value back from the cache, don’t assume it’s the right value or the right type.
4. Bugs can exist in the smallest of classes.
5. Test, Test, Test, and then Test.

It’s always fun finding these kinds of bugs because it means the apps we build will be better for it.