Throttling duplicate requests in an HttpModule

Question

I'm writing a method to throttle duplicate requests taking place within multiple HttpModule instances within a web application.

Currently, I have the following setup:

// Container for semaphores
private static readonly ConcurrentDictionary<string, SemaphoreSlim> 
SemaphoreSlims = new ConcurrentDictionary<string, SemaphoreSlim>();
// Wrapper for getting semaphore
private static SemaphoreSlim GetSemaphoreSlim(string id)
{
 return SemaphoreSlims.GetOrAdd(id, new SemaphoreSlim(1, 1));
}
private async Task ProcessImageAsync(HttpContext context)
{
 // `hash` is the request path hashed.
 SemaphoreSlim semaphore = GetSemaphoreSlim(hash);
 await semaphore.WaitAsync();
 try
 {
 // Do awaitable task
 }
 finally
 {
 semaphore.Release();
 }
}

This will work but I have no way of disposing of the semaphore instances since I need to preserve the ConcurrentDictionary and cannot dispose of a semaphore within the Dispose() method for that HttpModule instance as it could be currently waiting in another instance.

By my calculation it would take up ~156 megabytes of memory for 1 million items stored in the dictionary. It's not a massive amount but I'd really like it to be scalable and somehow find a way to clean up after myself.

Is this a sensible pattern and is there a way I can improve on it?

Answer 1

Semaphore has a bigger overhead than a traditional lock pattern, aside being too complex for what you need. You could use a class such as this:

public class DeDuper : IDisposable
{
 private static readonly ConcurrentDictionary<String, Object> currentRequests = new ConcurrentDictionary();
 private string _hash;
 public DeDuper(string hash)
 {
 _hash = hash;
 var lockable = currentRequests.GetOrAdd(hash, () => new Object());
 Monitor.Enter(lockable);
 }
 public void Dispose()
 {
 object lockable;
 if(currentRequests.TryGetValue(_hash, out lockable) && Monitor.IsEntered(lockable))
 {
 Monitor.Exit(lockable);
 }
 }
 ~DeDuper()
 {
 Dispose();
 }
}

Then your class can simply be written as:

private async Task ProcessImageAsync(HttpContext context)
{
 using(new DeDuper(hash))
 {
 // do awaitable task
 }
}

You will have a class that only allows a single hash to be run at any one time, with the added side effect of not having to release the Semaphore ;)

Not sure if this would work, i'd need to benchmark but, you could reduce the amount of memory further by reducing the commonality before you hash, eg:

http://mydomain.com/path1/part1 becomes path1/part1
http://mydomain.com/path1/part2 becomes path1/part2

Update

I thought that you might be able to use Interlocked.CompareExchange to do the job that you want, but it turns out AutoResetEvent works better. Here is a working sample:

Actual locking code

public class LockAcquisition<T> : IDisposable
{
 private static readonly ConcurrentDictionary<T, LockWrapper> currentRequests = new ConcurrentDictionary<T, LockWrapper>();
 private readonly LockWrapper _currentLock;
 public LockAcquisition(T key)
 {
 _currentLock = currentRequests.GetOrAdd(key, new LockWrapper());
 _currentLock.AcquireLock();
 }
 public void Dispose()
 {
 if (_currentLock != null)
 {
 _currentLock.Dispose();
 }
 }
 ~LockAcquisition()
 {
 Dispose();
 }
 private class LockWrapper : IDisposable
 {
 private readonly AutoResetEvent _locker = new AutoResetEvent(true);
 public void AcquireLock()
 {
 _locker.WaitOne();
 }
 public void Dispose()
 {
 _locker.Set();
 }
 ~LockWrapper()
 {
 Dispose();
 }
 }
}

Wrapped to use a string as the key

public class LockAcquisition : LockAcquisition<String>
{
 public LockAcquisition(string key)
 : base(key)
 {
 } 
}

Testing code

internal class Program
{
 private static void Main(string[] args)
 {
 Run();
 Console.ReadKey();
 }
 private static async void Run()
 {
 const string hash = "123456";
 const string hash2 = "1234567";
 using (new LockAcquisition(hash))
 {
 using (new LockAcquisition(hash2))
 {
 using (new LockAcquisition(hash)) // This will deadlocked
 { 
 }
 await Task.Delay(TimeSpan.FromSeconds(2));
 Console.WriteLine("Done 2");
 }
 await Task.Delay(TimeSpan.FromSeconds(2));
 Console.WriteLine("Done 1");
 }
 }
}

• James South: Thanks for the example, definitely a better direction. I'm fairly certain though that you can't use await within a lock though. See this. stackoverflow.com/questions/21404144/…
• Stuart Blackler: Ah, I didn't realise that you couldn't do that. I will test an alternative approach on lunch (approx 1 hour from now). My main point sticks though, wrap the locking code into a separate class that hides away the details of the locking mechanism.
• James South: Much appreciated. Yeah, definitely a good point. I'm gonna have a look also, maybe something based on the AsyncLock here. stackoverflow.com/questions/21011179/…
• Stuart Blackler: @JamesSouth hopefully my update should be better for you. I think it should work with async/await. Will test more later.