3

I want to place an authentication token in a cache where it can be used in multiple app domains. This token will expire every hour. When a client is unable to authorize with the token, it asks the token generation service to generate a new one.

I only want this regeneration to occur for the first client that does not authenticate sucessfully, so I've employed a lock object like this:

public async Task<Token> GenerateToken(Token oldToken)
{
    Token token;
    lock (lockObject)
    {
        var cachedToken = GetTokenFromCache();
        if (cachedToken == oldToken)
        {
            var authClient = new AuthClient(id, key);
            token = await authClient.AuthenticateClientAsync(); //KABOOM
            PutTokenInCache(token);
        }
        else
        {
            token = cachedToken;
        }
    }
    return token;
}

My issue is AuthClient only has async methods, and async methods are not allowed in lock-statement blocks. I don't have any control over AuthClient, is there some other strategy I can employ here?

Mister Epic
  • 16,295
  • 13
  • 76
  • 147

1 Answers1

8

You can use SemaphoreSlim as a basic async-ready lock replacement:

private readonly SemaphoreSlim lockObject = new SemaphoreSlim(1);
public async Task<Token> GenerateToken(Token oldToken)
{
  Token token;
  await lockObject.WaitAsync();
  try
  {
    var cachedToken = GetTokenFromCache();
    if (cachedToken == oldToken)
    {
      var authClient = new AuthClient(id, key);
      token = await authClient.AuthenticateClientAsync();
      PutTokenInCache(token);
    }
    else
    {
      token = cachedToken;
    }
  }
  finally
  {
    lockObject.Release();
  }
  return token;
}
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • This looks great! "The SemaphoreSlim class represents a lightweight, fast semaphore that can be used for waiting within a single process when wait times are expected to be very short." If it takes up to 5 seconds to retrieve the token, are there any issues with this? – Mister Epic Dec 01 '14 at 15:28
  • 1
    @MisterEpic: No. However, if your cache is in-memory, you could consider caching the *task* instead of the *token*, which would simplify the code a bit. – Stephen Cleary Dec 01 '14 at 15:30
  • 1
    The great thing about the `lock` statement is that it knows which thread it's being called from, so recursion inside a `lock` is not a problem. `SemaphoreSlim` however is recursion-incompatible so it's not a 100% replacement, be cautious people. – Alex from Jitbit Jan 26 '20 at 16:54
  • @Alex Good point. There are a few attempts out there to address re-entrance on `SemaphoreSlim`. Before over-using them, one should carefully examine if existing nested locks are necessary. That said, I highly recommend the [Flettu.AsyncLock](https://github.com/mysteryjeans/Flettu/blob/master/src/Flettu/Lock/AsyncLock.cs) class. It is clean and simple and he uses [AsyncLocal](https://learn.microsoft.com/en-us/dotnet/api/system.threading.asynclocal-1) which is native to the framework and carries state through an async flow the way `ThreadLocal` can do for synchronous code. – mdisibio Feb 22 '21 at 20:40
  • 1
    IMO, [re-entrant locks are a design mistake](https://blog.stephencleary.com/2013/04/recursive-re-entrant-locks.html) in the first place. – Stephen Cleary Feb 22 '21 at 23:45