Wrapping IMemoryCache with SemaphoreSlim I need simple caching for one of my web-services so I use the Microsoft.Extensions.Caching.Memory package. I wrapped it with SemaphoreSlim to make sure reading and writing are safe. I'm not sure what I'm doing here is entirely correct. What do you think? Is this wrapper OK or can I turn GetOrCreateAsync into an extension for IMemoryCache and it'll handle everything?

public class CacheService
{
    private static readonly SemaphoreSlim Locker = new SemaphoreSlim(1, 1);

    private readonly IMemoryCache _cache;

    public CacheService(IMemoryCache cache)
    {
        _cache = cache;
    }

    public async Task&lt;T&gt; GetOrCreateAsync&lt;T&gt;(object key, Func&lt;Task&lt;T&gt;&gt; create)
    {
        await Locker.WaitAsync();
        try
        {
            if (_cache.TryGetValue(key, out var entry))
            {
                return (T)entry;
            }
            else
            {
                return _cache.Set(key, await create(), new MemoryCacheEntryOptions
                {
                    AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(17)
                });
            }
        }
        finally
        {
            Locker.Release();
        }
    }
}
Sorry this is a bit of a ramble, but a few things jump out at me:


17?!?! I don't have to tell you about magic numbers! This should probably be configurable... somehow...
Why is Locker static? It doesn't make sense to restrict access to one cache because a totally different cache is currently in use.
Locking the cache while you create an item isn't necessarily going to end well: it would be all too easy for create to try to access the cache, and now you have a deadlock. The 'easiest' way to resolve this would be to slightly abuse the cache, and instead of storing the values themselves store some wrapper which indicates whether the value is written yet with a different concurrency mechanism. This way, you can add a 'pending' value to the cache immediately, which can be initialised outside the lock. 

This increases the complexity of dealing with create failures, because you have to communicate between threads with some other object, but now you can only dead-lock yourself by trying to access the object your are currently creating. It would be nice to detect this, but I think it might be impossible, so you probably want to give whatever mechanism awaits the pending entry a time-out. I don't think you can do better than this without passing the complexity off to the caller, which of course would ruin the nice API.

A better solution would probably be to provide a completely separate concurrency mechanism for handling 'pending' values, so that there is less overhead and complexity accessing the cache (presumably the more common operation). It wouldn't store the values, but rather just provide a mechanism to wait for them to appear in the proper cache (on a key-by-key basis).
I'd prefer that the cast to (T) was checked first, so that you can throw a highly-specific exception explaining that whatever was in the cache was not what the caller was expecting.
Since you are providing the access control, your class is probably also responsible for disposing the cache: it should provide this facility. Disposing Locker is also a concern. This adds to the opportunity for a consumer to get cryptic error messages if they are trying to access the cache as it is disposed.
Public APIs should have inline documentation (///), so that the maintainer knows what it's meant to do, and consumers can find out how to use them correctly without consulting the source-code.