HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Throttling duplicate requests in an HttpModule

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
duplicatethrottlinghttpmodulerequests

Problem

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 
SemaphoreSlims = new ConcurrentDictionary();

// 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?

Solution

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 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 : IDisposable
{
    private static readonly ConcurrentDictionary currentRequests = new ConcurrentDictionary();
    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
{
    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");
        }
    }
}

Code Snippets

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();
    }
}
private async Task ProcessImageAsync(HttpContext context)
{
    using(new DeDuper(hash))
    {
        // do awaitable task
    }
}
http://mydomain.com/path1/part1   becomes   path1/part1
http://mydomain.com/path1/part2   becomes   path1/part2
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();
        }

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

Context

StackExchange Code Review Q#58964, answer score: 2

Revisions (0)

No revisions yet.