Подтвердить что ты не робот

Блокировка по строкам. Является ли это безопасным/нормальным?

Мне нужно заблокировать раздел кода по строке. Конечно, следующий код ужасно опасен:

lock("http://someurl")
{
    //bla
}

Итак, я приготовил альтернативу. Обычно я не размещаю большие куски кода, но когда речь заходит о параллельном программировании, я немного опасаюсь создания собственной схемы синхронизации, поэтому я отправляю свой код, чтобы спросить, правильно ли это сделать таким образом, или существует ли более простой подход.

public class StringLock
{
    private readonly Dictionary<string, LockObject> keyLocks = new Dictionary<string, LockObject>();
    private readonly object keyLocksLock = new object();

    public void LockOperation(string url, Action action)
    {
        LockObject obj;
        lock (keyLocksLock)
        {
            if (!keyLocks.TryGetValue(url,
                                      out obj))
            {
                keyLocks[url] = obj = new LockObject();
            }
            obj.Withdraw();
        }
        Monitor.Enter(obj);
        try
        {
            action();
        }
        finally
        {
            lock (keyLocksLock)
            {
                if (obj.Return())
                {
                    keyLocks.Remove(url);
                }
                Monitor.Exit(obj);
            }
        }
    }

    private class LockObject
    {
        private int leaseCount;

        public void Withdraw()
        {
            Interlocked.Increment(ref leaseCount);
        }

        public bool Return()
        {
            return Interlocked.Decrement(ref leaseCount) == 0;
        }
    }
}

Я бы использовал его следующим образом:

StringLock.LockOperation("http://someurl",()=>{
    //bla
});

Хорошо, чтобы идти, или сбой и гореть?

ИЗМЕНИТЬ

Для потомков, здесь мой рабочий код. Спасибо за все предложения:

public class StringLock
{
    private readonly Dictionary<string, LockObject> keyLocks = new Dictionary<string, LockObject>();
    private readonly object keyLocksLock = new object();

    public IDisposable AcquireLock(string key)
    {
        LockObject obj;
        lock (keyLocksLock)
        {
            if (!keyLocks.TryGetValue(key,
                                      out obj))
            {
                keyLocks[key] = obj = new LockObject(key);
            }
            obj.Withdraw();
        }
        Monitor.Enter(obj);
        return new DisposableToken(this,
                                   obj);
    }

    private void ReturnLock(DisposableToken disposableLock)
    {
        var obj = disposableLock.LockObject;
        lock (keyLocksLock)
        {
            if (obj.Return())
            {
                keyLocks.Remove(obj.Key);
            }
            Monitor.Exit(obj);
        }
    }

    private class DisposableToken : IDisposable
    {
        private readonly LockObject lockObject;
        private readonly StringLock stringLock;
        private bool disposed;

        public DisposableToken(StringLock stringLock, LockObject lockObject)
        {
            this.stringLock = stringLock;
            this.lockObject = lockObject;
        }

        public LockObject LockObject
        {
            get
            {
                return lockObject;
            }
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        ~DisposableToken()
        {
            Dispose(false);
        }

        private void Dispose(bool disposing)
        {
            if (disposing && !disposed)
            {
                stringLock.ReturnLock(this);
                disposed = true;
            }
        }
    }

    private class LockObject
    {
        private readonly string key;
        private int leaseCount;

        public LockObject(string key)
        {
            this.key = key;
        }

        public string Key
        {
            get
            {
                return key;
            }
        }

        public void Withdraw()
        {
            Interlocked.Increment(ref leaseCount);
        }

        public bool Return()
        {
            return Interlocked.Decrement(ref leaseCount) == 0;
        }
    }
}

Используется следующим образом:

var stringLock=new StringLock();
//...
using(stringLock.AcquireLock(someKey))
{
    //bla
}
4b9b3361

Ответ 1

Другой вариант - получить HashCode каждого URL-адреса, затем разделить его на простое число и использовать его как индекс в виде массива блокировок. Это позволит ограничить количество блокировок, которые вам нужны, позволяя вам контролировать вероятность "ложной блокировки", выбрав количество используемых замков.

Однако вышеизложенное имеет смысл только в том случае, если он слишком дорогостоящий, имеет только одну блокировку для активного URL-адреса.

Ответ 2

Блокировка произвольным экземпляром строки была бы плохой идеей, потому что Monitor.Lock блокирует экземпляр. Если бы у вас было два разных экземпляра строк с одним и тем же контентом, это были бы две независимые блокировки, которые вам не нужны. Поэтому вы правы, чтобы беспокоиться о произвольных строках.

Однако .NET уже имеет встроенный механизм для возврата "канонического экземпляра" заданного содержимого строки: String.Intern. Если вы передадите ему два разных экземпляра строки с одним и тем же контентом, вы получите один и тот же экземпляр результата оба раза.

lock (string.Intern(url)) {
    ...
}

Это проще; там меньше кода для тестирования, потому что вы будете полагаться на то, что уже есть в Framework (что, по-видимому, уже работает).

Ответ 3

Вот очень простое, элегантное и правильное решение для .NET 4 с использованием ConcurrentDictionary, адаптированного из этого вопроса.

public static class StringLocker
{
    private static readonly ConcurrentDictionary<string, object> _locks = new ConcurrentDictionary<string, object>();

    public static void DoAction(string s, Action action)
    {
        lock(_locks.GetOrAdd(s, new object()))
        {
            action();
        }
    }
}

Вы можете использовать его так:

StringLocker.DoAction("http://someurl", () =>
{
    ...
});

Ответ 4

Около 2 лет назад мне пришлось реализовать одно и то же:

Я пытаюсь написать кэш изображения где несколько клиентов (более 100 или поэтому), скорее всего, запросят изображение одновременно. Первый хит запросить изображение с другого сервера, и я хотел бы, чтобы все остальные запросы кэша до тех пор, пока эта операция не будет чтобы они могли получить кешированная версия.

Я закончил с кодом, делающим то же самое, что и ваш (словарь LockObjects). Ну, ваш, кажется, лучше инкапсулирован.

Итак, я думаю, у вас есть неплохое решение. Всего 2 комментария:

  • Если вам нужна еще лучшая форма, возможно, полезно использовать какой-то ReadWriteLock, так как у вас есть 100 читателей и только один писатель получает изображение с другого сервера.

  • Я не уверен, что произойдет в случае переключения потока непосредственно перед Monitor.Enter(obj); в вашем LockOperation(). Скажем, первый поток, требующий изображения, строит новую блокировку и затем переключает поток непосредственно перед тем, как он войдет в критический раздел. Тогда может случиться, что вторая нить входит в критический раздел перед первым. Ну, может быть, это не настоящая проблема.

Ответ 5

Вы создали довольно сложную оболочку вокруг простого оператора блокировки. Не было бы лучше создать словарь url и создать объект блокировки для каждого и каждого. Вы могли бы просто сделать.

objLink = GetUrl("Url"); //Returns static reference to url/lock combination
lock(objLink.LockObject){
    //Code goes here
}

Вы могли бы даже упростить это, заблокировав объект objLink, который может быть экземпляром строки GetUrl ( "Url" ). (вам придется заблокировать статический список строк, хотя)

Вы являетесь исходным кодом, если он подвержен ошибкам. Если код:

if (obj.Return())
{
keyLocks.Remove(url);
}

Если исходный код finally генерирует исключение, вы остаетесь с недопустимым состоянием LockObject.

Ответ 6

Я добавил решение в Bardock.Utils пакет, вдохновленный @eugene-beresovsky answer.

Использование:

private static LockeableObjectFactory<string> _lockeableStringFactory = 
    new LockeableObjectFactory<string>();

string key = ...;

lock (_lockeableStringFactory.Get(key))
{
    ...
}

Код решения:

namespace Bardock.Utils.Sync
{
    /// <summary>
    /// Creates objects based on instances of TSeed that can be used to acquire an exclusive lock.
    /// Instanciate one factory for every use case you might have.
    /// Inspired by Eugene Beresovsky solution: https://stackoverflow.com/a/19375402
    /// </summary>
    /// <typeparam name="TSeed">Type of the object you want lock on</typeparam>
    public class LockeableObjectFactory<TSeed>
    {
        private readonly ConcurrentDictionary<TSeed, object> _lockeableObjects = new ConcurrentDictionary<TSeed, object>();

        /// <summary>
        /// Creates or uses an existing object instance by specified seed
        /// </summary>
        /// <param name="seed">
        /// The object used to generate a new lockeable object.
        /// The default EqualityComparer<TSeed> is used to determine if two seeds are equal. 
        /// The same object instance is returned for equal seeds, otherwise a new object is created.
        /// </param>
        public object Get(TSeed seed)
        {
            return _lockeableObjects.GetOrAdd(seed, valueFactory: x => new object());
        }
    }
}

Ответ 7

Вот как я реализовал эту схему блокировки:

public class KeyLocker<TKey>
{
    private class KeyLock
    {
        public int Count;
    }

    private readonly Dictionary<TKey, KeyLock> keyLocks = new Dictionary<TKey, KeyLock>();

    public T ExecuteSynchronized<T>(TKey key, Func<TKey, T> function)
    {
        KeyLock keyLock =  null;
        try
        {              
            lock (keyLocks)
            {
                if (!keyLocks.TryGetValue(key, out keyLock))
                {
                    keyLock = new KeyLock();
                    keyLocks.Add(key, keyLock);
                }
                keyLock.Count++;
            }
            lock (keyLock)
            {
                return function(key);
            }
        }
        finally
        {         
            lock (keyLocks)
            {
                if (keyLock != null && --keyLock.Count == 0) keyLocks.Remove(key);
            }
        }
    }

    public void ExecuteSynchronized(TKey key, Action<TKey> action)
    {
        this.ExecuteSynchronized<bool>(key, k =>
        {
            action(k);
            return true;
        });
    }
}

И используется так:

private locker = new KeyLocker<string>();
......

void UseLocker()
{
     locker.ExecuteSynchronized("some string", () => DoSomething());
}

Ответ 8

В большинстве случаев, когда вы считаете, что вам нужны блокировки, вы этого не делаете. Вместо этого попробуйте использовать потокобезопасную структуру данных (например, Queue), которая обрабатывает блокировку для вас.

Например, в python:

class RequestManager(Thread):
    def __init__(self):
        super().__init__()
        self.requests = Queue()
        self.cache = dict()
    def run(self):        
        while True:
            url, q = self.requests.get()
            if url not in self.cache:
                self.cache[url] = urlopen(url).read()[:100]
            q.put(self.cache[url])

    # get() runs on MyThread thread, not RequestManager's
    def get(self, url):
        q = Queue(1)
        self.requests.put((url, q))
        return q.get()

class MyThread(Thread):
    def __init__(self):
        super().__init__()
    def run(self):
        while True:
            sleep(random())
            url = ['http://www.google.com/', 'http://www.yahoo.com', 'http://www.microsoft.com'][randrange(0, 3)]
            img = rm.get(url)
            print(img)

Ответ 9

Теперь у меня такая же проблема. Я решил использовать простое решение: чтобы избежать взаимоблокировок, создайте новую строку с фиксированным уникальным префиксом и используйте ее как объект блокировки:

lock(string.Intern("uniqueprefix" + url))
{
//
}