Почему блокировка (это) {...} плохая?

Question

MSDN documentation говорит, что

public class SomeObject 
{ 
    public void SomeOperation() 
    { 
    lock(this) 
    { 
     //Access instance variables 
    } 
    } 
} 

является «проблемой, если экземпляр может быть доступен публично». Мне интересно, почему? Это потому, что замок будет удерживаться дольше, чем необходимо? Или есть какая-то более коварная причина?

Answer 1

Это плохой форме использовать this в операторах блокировки, потому что, как правило, вне вашего контроля, кто-то может быть запирающего на этом объекте.

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

Частное поле, как правило, является лучшим вариантом, поскольку компилятор будет применять к нему ограничения доступа, и он будет инкапсулировать механизм блокировки. Использование this нарушает инкапсуляцию, подвергая часть реализации блокировки публике. Также не ясно, что вы приобретете замок на this, если он не был задокументирован. Даже тогда, полагаясь на документацию, чтобы предотвратить проблему, является субоптимальным.

Наконец, существует общее заблуждение, что lock(this) фактически изменяет объект, переданный как параметр, и каким-то образом делает его доступным только для чтения или недоступным. Это false. Объект, переданный как параметр lock, служит только как ключ. Если блокировка уже удерживается на этом ключе, блокировка не может быть выполнена; в противном случае блокировка разрешена.

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

В качестве примера выполните следующий код C#.

public class Person 
{ 
    public int Age { get; set; } 
    public string Name { get; set; } 

    public void LockThis() 
    { 
     lock (this) 
     { 
      System.Threading.Thread.Sleep(10000); 
     } 
    } 
} 

class Program 
{ 
    static void Main(string[] args) 
    { 
     var nancy = new Person {Name = "Nancy Drew", Age = 15}; 
     var a = new Thread(nancy.LockThis); 
     a.Start(); 
     var b = new Thread(Timewarp); 
     b.Start(nancy); 
     Thread.Sleep(10); 
     var anotherNancy = new Person { Name = "Nancy Drew", Age = 50 }; 
     var c = new Thread(NameChange); 
     c.Start(anotherNancy); 
     a.Join(); 
     Console.ReadLine(); 
    } 

    static void Timewarp(object subject) 
    { 
     var person = subject as Person; 
     if (person == null) throw new ArgumentNullException("subject"); 
     // A lock does not make the object read-only. 
     lock (person.Name) 
     { 
      while (person.Age <= 23) 
      { 
       // There will be a lock on 'person' due to the LockThis method running in another thread 
       if (Monitor.TryEnter(person, 10) == false) 
       { 
        Console.WriteLine("'this' person is locked!"); 
       } 
       else Monitor.Exit(person); 
       person.Age++; 
       if(person.Age == 18) 
       { 
        // Changing the 'person.Name' value doesn't change the lock... 
        person.Name = "Nancy Smith"; 
       } 
       Console.WriteLine("{0} is {1} years old.", person.Name, person.Age); 
      } 
     } 
    } 

    static void NameChange(object subject) 
    { 
     var person = subject as Person; 
     if (person == null) throw new ArgumentNullException("subject"); 
     // You should avoid locking on strings, since they are immutable. 
     if (Monitor.TryEnter(person.Name, 30) == false) 
     { 
      Console.WriteLine("Failed to obtain lock on 50 year old Nancy, because Timewarp(object) locked on string \"Nancy Drew\"."); 
     } 
     else Monitor.Exit(person.Name); 

     if (Monitor.TryEnter("Nancy Drew", 30) == false) 
     { 
      Console.WriteLine("Failed to obtain lock using 'Nancy Drew' literal, locked by 'person.Name' since both are the same object thanks to inlining!"); 
     } 
     else Monitor.Exit("Nancy Drew"); 
     if (Monitor.TryEnter(person.Name, 10000)) 
     { 
      string oldName = person.Name; 
      person.Name = "Nancy Callahan"; 
      Console.WriteLine("Name changed from '{0}' to '{1}'.", oldName, person.Name); 
     } 
     else Monitor.Exit(person.Name); 
    } 
} 

Консоль вывода

'this' person is locked! 
Nancy Drew is 16 years old. 
'this' person is locked! 
Nancy Drew is 17 years old. 
Failed to obtain lock on 50 year old Nancy, because Timewarp(object) locked on string "Nancy Drew". 
'this' person is locked! 
Nancy Smith is 18 years old. 
'this' person is locked! 
Nancy Smith is 19 years old. 
'this' person is locked! 
Nancy Smith is 20 years old. 
Failed to obtain lock using 'Nancy Drew' literal, locked by 'person.Name' since both are the same object thanks to inlining! 
'this' person is locked! 
Nancy Smith is 21 years old. 
'this' person is locked! 
Nancy Smith is 22 years old. 
'this' person is locked! 
Nancy Smith is 23 years old. 
'this' person is locked! 
Nancy Smith is 24 years old. 
Name changed from 'Nancy Drew' to 'Nancy Callahan'. 

Answer 2

Потому что, если люди могут получить указатель объекта (то есть: ваш this) указатель, то они также могут попытаться заблокировать тот же объект. Теперь они могут не знать, что вы замок на this внутренне, так что это может вызвать проблемы (возможно, затор)

В дополнение к этому, это также плохая практика, потому что это замок «слишком много»

Например, у вас может быть переменная-член из List<int>, и единственное, что вам нужно заблокировать, это переменная-член. Если вы заблокируете весь объект в своих функциях, то другие функции, вызывающие эти функции, будут заблокированы в ожидании блокировки. Если эти функции не нуждаются в доступе к списку участников, вы будете вынуждать другой код ждать и замедлять работу приложения без каких-либо оснований.

Answer 3

... и точно такие же аргументы применимы и к этой конструкции, а также:

lock(typeof(SomeObject)) 

Answer 4

Посмотрите на MSDN Тема Thread Synchronization (C# Programming Guide)

Как правило, лучше всего, чтобы избежать блокировки на публичный тип или объект экземпляров, находящихся вне контроля вашего приложения . Например, блокировка (это) может быть проблематичной, если экземпляр может получить доступ публично, потому что код вне вашего контроля может также заблокировать объект . Это может создать тупиковые ситуации, когда два или более потоков ждут выхода того же объекта. Блокировка на публичном типе данных, в отличие от объекта, может вызвать проблемы для той же причины . Блокировка на литеральных строках особенно рискованна, потому что строки строки интернационализированы общей версией языка (CLR). Это означает, что существует один экземпляр любого данного строкового литерала для всей программы , точный же объект представляет собой буквальный пробел доменов приложений, всех тем. В результате блокировка, помещенная на строку с тем же содержимым в любом месте процесса приложения , блокирует все экземпляры этой строки в приложении . В результате лучше всего заблокировать частный или защищенный член , который не интернирован. Некоторые классы обеспечивают элементы, специально предназначенные для блокировки . Тип массива, например, , предоставляет SyncRoot. Многие типы коллекций предоставляют элемент SyncRoot как нуль .

Answer 5

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

Чтобы быть ясным, это плохо, потому что некоторые другие фрагменты кода могут использовать экземпляр класса для блокировки и могут помешать вашему коду получить своевременную блокировку или могут создать другие проблемы синхронизации потоков. Лучший случай: ничто другое не использует ссылку на ваш класс для блокировки. Средний случай: что-то использует ссылку на ваш класс, чтобы делать блокировки, и это вызывает проблемы с производительностью. Худший случай: что-то использует ссылку вашего класса, чтобы делать блокировки, и это вызывает действительно плохие, очень тонкие, действительно трудно отлаживаемые проблемы.

Answer 6

Там также некоторые хорошая дискуссия об этом здесь: Is this the proper use of a mutex?

Answer 7

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

• Вы не можете отменить тупиковую ситуацию, не прерывая один из потоков, так после вы получите в тупик, вы не можете выйти
• Голодание закончится автоматически после того, как один из потоков заканчивает свою работу

Here является изображение, которое иллюстрирует разницу.

Заключение
Вы все еще можете безопасно использовать lock(this) если нить голодание не является проблемой для вас. Вам все равно нужно иметь в виду, что когда нить, которая является голодной нитью с использованием lock(this), заканчивается блокировкой, в которой ваш объект заблокирован, он, наконец, заканчивается вечным голоданием;)

Answer 8

Я знаю, что это старая нить, а потому, люди все еще могут смотреть на это и полагаться на него, важно отметить, что lock(typeof(SomeObject)) значительно хуже, чем lock(this). Было сказано, что; искреннее признание Алану за то, что он указал, что lock(typeof(SomeObject)) - плохая практика.

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

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

Но lock(typeof(SomeObject)) открывает совершенно новую и усиленную банку червей.

Для чего это стоит.

Answer 9

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

Answer 10

Ниже приведен пример кода, который проще следовать (ИМО): (Будет работать в LINQPad, эталонные следующие пространства имен: System.Net и System.Threading.Tasks)

void Main() 
{ 
    ClassTest test = new ClassTest(); 
    lock(test) 
    { 
     Parallel.Invoke (
      () => test.DoWorkUsingThisLock(1), 
      () => test.DoWorkUsingThisLock(2) 
     ); 
    } 
} 

public class ClassTest 
{ 
    public void DoWorkUsingThisLock(int i) 
    { 
     Console.WriteLine("Before ClassTest.DoWorkUsingThisLock " + i); 
     lock(this) 
     { 
      Console.WriteLine("ClassTest.DoWorkUsingThisLock " + i); 
      Thread.Sleep(1000); 
     } 
     Console.WriteLine("ClassTest.DoWorkUsingThisLock Done " + i); 
    } 
} 

Answer 11

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

Поскольку забота об обмене, ваш менеджер решает, что клиенты могут напрямую использовать секретаря. Но это имеет побочный эффект: клиент может даже потребовать их, пока вы работаете с этим клиентом, и вам также понадобится выполнить часть задач. Замыкает тупик, потому что требование больше не является иерархией. Этого можно избежать, если не позволить клиентам требовать их в первую очередь.

lock(this) Плохо, как мы видели. Внешний объект может блокировать объект, и поскольку вы не контролируете, кто использует этот класс, любой может заблокировать его ... Какой именно пример, как описано выше. Опять же, решение заключается в ограничении воздействия объекта. Однако, если у вас есть private, protected или internal класс, то вы, , уже можете контролировать, кто блокирует ваш объект, потому что вы уверены, что сами написали свой код. Итак, сообщение здесь: не подвергайте его public. Кроме того, обеспечение того, чтобы блокировка использовалась в аналогичном сценарии, позволяет избежать взаимоблокировок.

Полная противоположность этому заключается в том, чтобы блокировать ресурсы, общие для всего домена приложения - наихудший сценарий. Это похоже на то, чтобы выставлять свою секретаршу на улицу и позволять всем там требовать их. Результатом является полный хаос - или с точки зрения исходного кода: это была плохая идея; выбросьте его и начните. Итак, как мы это делаем?

Типы разделяются в домене приложения, так как большинство людей здесь указывают. Но есть даже лучшие вещи, которые мы можем использовать: строки. Причина в том, что строки объединены. Другими словами: если у вас есть две строки, которые имеют одинаковое содержимое в домене приложения, есть вероятность, что они имеют тот же самый указатель. Поскольку указатель используется как ключ блокировки, то, что вы в основном получаете, является синонимом «готовьтесь к неопределенному поведению».

Аналогично, вы не должны блокировать объекты WCF, HttpContext.Current, Thread.Current, Singletons (в общем) и т. Д. Самый простой способ избежать всего этого? private [static] object myLock = new object();

Answer 12

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

Проверьте этот код, чтобы узнать, что я имею в виду. Добавьте следующий код в свою основную программу в консольном приложении:

static void Main(string[] args) 
    { 
     TestThreading(); 
     Console.ReadLine(); 
    } 

    public static void TestThreading() 
    { 
     Random rand = new Random(); 
     Thread[] threads = new Thread[10]; 
     TestLock.balance = 100000; 
     for (int i = 0; i < 10; i++) 
     { 
      TestLock tl = new TestLock(); 
      Thread t = new Thread(new ThreadStart(tl.WithdrawAmount)); 
      threads[i] = t; 
     } 
     for (int i = 0; i < 10; i++) 
     { 
      threads[i].Start(); 
     } 
     Console.Read(); 
    } 

Создайте новый класс, как показано ниже.

class TestLock 
{ 
    public static int balance { get; set; } 
    public static readonly Object myLock = new Object(); 

    public void Withdraw(int amount) 
    { 
     // Try both locks to see what I mean 
     //    lock (this) 
     lock (myLock) 
     { 
      Random rand = new Random(); 
      if (balance >= amount) 
      { 
       Console.WriteLine("Balance before Withdrawal : " + balance); 
       Console.WriteLine("Withdraw  : -" + amount); 
       balance = balance - amount; 
       Console.WriteLine("Balance after Withdrawal : " + balance); 
      } 
      else 
      { 
       Console.WriteLine("Can't process your transaction, current balance is : " + balance + " and you tried to withdraw " + amount); 
      } 
     } 

    } 
    public void WithdrawAmount() 
    { 
     Random rand = new Random(); 
     Withdraw(rand.Next(1, 100) * 100); 
    } 
} 

Вот пробег блокировки программы на это.

Balance before Withdrawal : 100000 
    Withdraw  : -5600 
    Balance after Withdrawal : 94400 
    Balance before Withdrawal : 100000 
    Balance before Withdrawal : 100000 
    Withdraw  : -5600 
    Balance after Withdrawal : 88800 
    Withdraw  : -5600 
    Balance after Withdrawal : 83200 
    Balance before Withdrawal : 83200 
    Withdraw  : -9100 
    Balance after Withdrawal : 74100 
    Balance before Withdrawal : 74100 
    Withdraw  : -9100 
    Balance before Withdrawal : 74100 
    Withdraw  : -9100 
    Balance after Withdrawal : 55900 
    Balance after Withdrawal : 65000 
    Balance before Withdrawal : 55900 
    Withdraw  : -9100 
    Balance after Withdrawal : 46800 
    Balance before Withdrawal : 46800 
    Withdraw  : -2800 
    Balance after Withdrawal : 44000 
    Balance before Withdrawal : 44000 
    Withdraw  : -2800 
    Balance after Withdrawal : 41200 
    Balance before Withdrawal : 44000 
    Withdraw  : -2800 
    Balance after Withdrawal : 38400 

Вот пробег блокировки программы на myLock.

Balance before Withdrawal : 100000 
Withdraw  : -6600 
Balance after Withdrawal : 93400 
Balance before Withdrawal : 93400 
Withdraw  : -6600 
Balance after Withdrawal : 86800 
Balance before Withdrawal : 86800 
Withdraw  : -200 
Balance after Withdrawal : 86600 
Balance before Withdrawal : 86600 
Withdraw  : -8500 
Balance after Withdrawal : 78100 
Balance before Withdrawal : 78100 
Withdraw  : -8500 
Balance after Withdrawal : 69600 
Balance before Withdrawal : 69600 
Withdraw  : -8500 
Balance after Withdrawal : 61100 
Balance before Withdrawal : 61100 
Withdraw  : -2200 
Balance after Withdrawal : 58900 
Balance before Withdrawal : 58900 
Withdraw  : -2200 
Balance after Withdrawal : 56700 
Balance before Withdrawal : 56700 
Withdraw  : -2200 
Balance after Withdrawal : 54500 
Balance before Withdrawal : 54500 
Withdraw  : -500 
Balance after Withdrawal : 54000 

Answer 13

Существует очень хорошая статья об этом http://bytes.com/topic/c-sharp/answers/249277-dont-lock-type-objects от Rico Mariani, производительность архитектора для выполнения Microsoft® .NET

Выдержки:

Основная проблема здесь состоит в том, что вы этого не сделаете собственный объект типа, а вы не знаете, кто еще мог бы получить к нему доступ. В общем, это очень плохая идея , чтобы полагаться на блокировку объекта, который вы не создали, и не знаете, кому еще может быть . Это делает тупик. Самый безопасный способ - блокировать только частные объекты.

Answer 14

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

http://blogs.msdn.com/b/bclteam/archive/2004/01/20/60719.aspx

Так решение добавить закрытый объект, например, lockObject к классу и поместить код региона внутри заявление замка, как показано ниже:

lock (lockObject) 
{ 
... 
}