Return to Answer

Updated with an alternative approach from comments

Source Link

edited Aug 4, 2014 at 13:18

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

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

Source Link

answered Aug 4, 2014 at 7:55

Stuart Blackler

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