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?

Stuart Blackler · Accepted Answer · 2014-08-04 13:18:15Z

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");
        }
    }
}

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/… — James South
– James South, Commented Aug 4, 2014 at 8:59
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. — Stuart Blackler
– Stuart Blackler, Commented Aug 4, 2014 at 9:46
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/… — James South
– James South, Commented Aug 4, 2014 at 10:27
@JamesSouth hopefully my update should be better for you. I think it should work with async/await. Will test more later. — Stuart Blackler
– Stuart Blackler, Commented Aug 4, 2014 at 13:21

Stack Exchange Network

Throttling duplicate requests in an HttpModule

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Throttling duplicate requests in an HttpModule

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions