6

I need to use IDistributedCache in my app and it could be MemoryDistributedCache or RedisCache as an underlying implementation. My understanding those are not thread-safe, so I created my own wrapper with usage of ReaderWriterLockSlim.

ReaderWriterLockSlim allows multiple threads for reading or exclusive access for writing. And now I wonder if it's the right tool for the job and if MemoryDistributedCache and RedisCache reading methods are thread-safe.

As to overall solution - I understand it takes time to store and read values from a distributed cache like Redis. But I don't have much choice as the values I store are even slower to get and manage.

Here's the snippet:

...
        public async Task<byte[]> GetAsync(string key, CancellationToken token = new CancellationToken())
        {
            _cacheLock.EnterReadLock();
            try
            {
                return await _cache.GetAsync(GetCacheKey(key), token);
            }
            finally
            {
                _cacheLock.ExitReadLock();
            }
        }

        public async Task SetAsync(string key, byte[] value, DistributedCacheEntryOptions options, CancellationToken token = new CancellationToken())
        {
            _cacheLock.EnterWriteLock();
            try
            {
                await _cache.SetAsync(GetCacheKey(key), value, options, token);
            }
            finally
            {
                _cacheLock.ExitWriteLock();
            }
        }

        public async Task RemoveAsync(string key, CancellationToken token = new CancellationToken())
        {
            _cacheLock.EnterWriteLock();
            try
            {
                await _cache.RemoveAsync(GetCacheKey(key), token);
            }
            finally
            {
                _cacheLock.ExitWriteLock();
            }
        }
AlexB
  • 4,167
  • 4
  • 45
  • 117
  • `MemoryDistributedCache` is thread safe since using MemoryCache internally. https://github.com/dotnet/extensions/blob/master/src/Caching/Memory/src/MemoryDistributedCache.cs#L34 –  Mar 21 '20 at 20:04
  • Thank you! But what about RedisCache? – AlexB Mar 21 '20 at 20:17
  • `RedisCache` is not thread-safe. They uses `IDatabase` as a singleton. https://github.com/dotnet/extensions/blob/master/src/Caching/StackExchangeRedis/src/RedisCache.cs#L166 –  Mar 21 '20 at 20:23
  • ok, so I'm going to the right direction then. Or... not sure about reads. – AlexB Mar 21 '20 at 21:02
  • I guess I'll need some tests – AlexB Mar 21 '20 at 21:03
  • The StackExchange.Redis docu says: "The object returned from GetDatabase is a cheap pass-thru object, and does not need to be stored." So I would say the implementation of `RedisCache` is not quite right. –  Mar 21 '20 at 21:08
  • 3
    IMO all implementations of `IDistributedCache` should be thread-safe. –  Mar 21 '20 at 21:21
  • 2
    I guess I was wrong. `IDatabase` is thread safe. https://stackoverflow.com/a/52945926/8810910 –  Mar 21 '20 at 22:28

1 Answers1

7

All IDistributedCache implementations should be thread-safe. If one is not, then please create a bug report to get it fixed.

Even if one of them is not thread-safe, using ReaderWriterLockSlim to guard async code is invalid. From the docs:

ReaderWriterLockSlim has managed thread affinity; that is, each Thread object must make its own method calls to enter and exit lock modes. No thread can change the mode of another thread.

The simplest workaround would be to use SemaphoreSlim instead of ReaderWriterLockSlim.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810