Является ли блокировка запрошенного объекта плохой идеей? - PullRequest
1 голос
/ 24 мая 2010

Большинство советов по безопасности потоков включают в себя некоторые варианты следующего шаблона:

public class Thing
{
    private readonly object padlock = new object();

    private IDictionary stuff, andNonsense;

    public Thing()
    {
        this.stuff = new Dictionary();
        this.andNonsense = new Dictionary();
    }

    public IDictionary Stuff
    {
        get
        {
            lock (this.padlock)
            {
                if (this.stuff.Count == 0)
                    this.stuff = this.SomeExpensiveOperation();
                return this.stuff;
            }
        }
    }

    public IDictionary AndNonsense
    {
        get
        {
            lock (this.padlock)
            {
                if (this.andNonsense.Count == 0)
                    this.andNonsense = this.AnotherExpensiveOperation();
                return this.andNonsense;
            }
        }
    }
    // Rest of class...
}

В тех случаях, когда операции get являются дорогостоящими и не связаны, один объект блокировки не подходит, поскольку вызов Stuff блокирует все вызовы AndNonsense, что снижает производительность. И вместо того, чтобы создавать объект блокировки для каждого вызова, не лучше ли получить блокировку для самого члена (при условии, что это не то, что реализует SyncRoot или что-то подобное для этой цели? Например:

public IDictionary Stuff
{
    get
    {
        lock (this.stuff)
        {
            if (this.stuff.Count == 0)
                this.stuff = this.SomeExpensiveOperation();
            return this.stuff;
        }
    }
}

Странно, я никогда не видел, чтобы этот подход рекомендовался или предупреждался против. Я что-то упускаю из виду?

РЕДАКТИРОВАТЬ 24 мая 2010 г.

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

Сводка правок:

  • Блокирующий объект в первом примере больше не является статическим (мой исходный пример был из статического класса).
  • Поля / свойства больше не являются строкой и инициализируются в конструкторе, поэтому никогда не равны нулю.
  • Перемещенный оператор возврата внутри блока lock{...}.

Ответы [ 6 ]

4 голосов
/ 24 мая 2010

Ваша версия «стандартного совета» была бы очень плохой - она ​​использовала бы одну блокировку для всего AppDomain. Обычно объект блокировки - это переменная instance , а не статическая.

Теперь, что касается вашего предложения: еще худшая идея, ИМО - потому что вы не знаете, что еще будет на нем зависеть. Некоторый другой код может использовать ту же строку - и аналогично блокировать ее. Теперь предположим, что два потока получают две блокировки вместо одной. Когда вы не представляете, что к чему, вы не надеетесь избежать тупика, кроме глупой удачи. (Есть также проблема недействительности, упомянутая codeka.)

Кроме того, это начинает сбивать с толку, если значение переменной изменяется - тогда вы потенциально могли бы одновременно выполнять два потока внутри блока блокировки, чего, вероятно, вы пытались избежать.

Везде, где возможно, вы должны (IMO) блокировать то, о чем знает только ваш код. Хотя это само по себе не сделает вас тупиком свободным или безопасным, это хорошая отправная точка, так как это значительно упрощает анализ вашего кода. Нечего и говорить, что для каждого экземпляра должна быть установлена ​​одна блокировка - вы можете создавать более тонкие блокировки, если хотите, - но, если это вообще возможно, держать их закрытыми для себя.

3 голосов
/ 24 мая 2010

В этом сценарии я бы использовал ReaderWriterLockSlim, который позволяет вам иметь несколько перекрывающихся блокировок «Чтение», только полностью блокируя, когда вы вводите блокировку «Запись» либо напрямую, либо через блокировку обновления. В качестве альтернативы, вы можете отключиться от блокировки, используя Thread.MemoryBarrier, но это сложно, так как продвинутая техника потребует некоторых специализированных тестов, чтобы убедиться, что она действительно работает.

3 голосов
/ 24 мая 2010

Что ж, я предполагаю, что вы на самом деле не пробовали этого, потому что lock(null) выдаст ArgumentNullException, так что вы не сможете так сделать.

Вы можете использовать отдельный объект "stuffLock", например так:

class Thing
{
    private string stuff;
    private object stuffLock = new Object();

    public string Stuff
    {
        get
        {
            lock(stuffLock)
            {
                if (stuff == null)
                    stuff = "Still threadsafe";
            }
            return stuff;
        }
    }
}
1 голос
/ 24 мая 2010

Обратите внимание, что ваши примеры не искробезопасны, так как вы запрашиваете значение переменной, которую пытаетесь защитить от вне блокировки.

Другой поток может получить фрагмент, сразу после того, как первый поток, запустив Stuff {get;}, снимет блокировку и снова установит значение this.stuff в значение null, прежде чем первый поток фактически вернет this.stuff

0 голосов
/ 24 мая 2010

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

0 голосов
/ 24 мая 2010

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

...