Pourquoi est-lock(this) {...} mauvais?

C'est mauvais pour utiliser this dans les instructions lock parce qu'il est généralement hors de votre contrôle qui d'autre pourrait être de verrouillage sur cet objet.

Afin de planifier correctement les opérations en parallèle, des précautions particulières doivent être prises afin d'envisager d'éventuelles situations de blocage, et d'avoir un nombre inconnu de verrouiller les points d'entrée qui empêche ce. Par exemple, l'une avec une référence à l'objet peut se verrouiller sur elle, sans que l'objet concepteur/créateur de savoir à ce sujet. Cela augmente la complexité de multi-thread solutions et peut nuire à leur exactitude.

Un champ privé est généralement une meilleure option que le compilateur va appliquer des restrictions d'accès, et il va encapsuler le mécanisme de verrouillage. À l'aide de this viole l'encapsulation en exposant une partie de votre verrouillage de la mise en œuvre au public. Il n'est pas aussi clair que vous fera l'acquisition d'un verrou sur this, sauf si elle a été documentée. Même puis, en s'appuyant sur la documentation de prévenir un problème est sous-optimale.

Enfin, il existe une idée fausse commune que lock(this) modifie en fait l'objet passé en paramètre, et d'une certaine façon fait en lecture seule ou inaccessibles. C'est faux. L'objet passé en paramètre à lock sert seulement à clé. Si un verrou est déjà en cours à cette clé, la serrure ne peut pas être fait; sinon, la serrure est autorisé.

C'est pourquoi c'est mauvais d'utiliser les chaînes de caractères comme les clés dans lock états, car ils sont immuables et sont partagés/accessible à l'ensemble des parties de l'application. Vous devez utiliser une variable privée au lieu de cela, un Object instance fera très bien l'affaire.

Exécuter le code C# suivant comme exemple.

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

Sortie de la Console

'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'.

Prendre un coup d'oeil à la Rubrique MSDN La Synchronisation Des Threads (Guide De Programmation C#)

Généralement, il est préférable d'éviter le blocage de
sur un type de public, ou sur l'objet
les instances au-delà du contrôle de votre
application. Par exemple, lock(cet)
peut être problématique si l'instance peut
être accessible au public, parce que le code
au-delà de votre contrôle peut se verrouiller sur l'
objet ainsi. Cela pourrait créer
les situations de blocage où deux ou plus
fils d'attente pour la libération de la
même objet. Le verrouillage d'un public
type de données, par opposition à un objet,
peut causer des problèmes pour la même
raison. Le verrouillage des chaînes de caractères littérales est
particulièrement dangereux, car littérale
les chaînes sont internés par la commune
language runtime (CLR). Cela signifie
qu'il y est une instance de n'importe quel
étant donné littéral de chaîne pour l'ensemble de la
programme, exactement le même objet
représente le littéral dans toutes les
les domaines d'application, sur tous les fils.
En conséquence, un verrou placé sur une chaîne
avec le même contenu partout dans le
processus de demande de serrures de tous
les instances de la chaîne dans les
application. En conséquence, il est préférable
pour verrouiller un privé ou protégé membre
ce n'est pas interné. Certaines classes
fournir aux membres spécifiquement pour
verrouillage. Le type du Tableau, par exemple,
fournit SyncRoot. Beaucoup de collection
types de fournir un SyncRoot membre
bien.

...et exactement les mêmes arguments s'appliquent à cette construction ainsi:

lock(typeof(SomeObject))

De verrouillage sur le ce pointeur peut être mauvais si vous êtes de verrouillage sur une ressource partagée. Une ressource partagée peut être une variable statique ou un fichier sur votre ordinateur, c'est à dire quelque chose qui est partagé entre tous les utilisateurs de la classe. La raison en est que le pointeur this contiendra une référence différente pour un emplacement en mémoire à chaque fois que votre classe est instanciée. Donc, le verrouillage sur ce à la fois instance d'une classe est différente de verrouillage sur ce dans une autre instance d'une classe.

Découvrez ce code pour voir ce que je veux dire. Ajoutez le code suivant à votre programme principal dans une application Console:

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

Créer une nouvelle classe comme ci-dessous.

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

Ici est un parcours du programme de verrouillage sur ce.

   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

Ici est un parcours du programme de verrouillage sur 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

Il y a aussi quelques bonnes discussion à ce sujet ici: Est-ce la bonne utilisation d'un mutex?

Parce que tout morceau de code qui peut voir l'instance de votre classe peut également verrouiller sur cette référence. Vous souhaitez cacher (encapsuler) votre objet de verrouillage, de sorte que seul code, qui doit faire référence, il peut faire référence à elle. Le mot-clé this fait référence à la classe en cours d'instance, de sorte que n'importe quel nombre de choses pourrait faire référence à celui-ci et pourrait l'utiliser pour faire la synchronisation des threads.

Pour être clair, ce est mauvais parce qu'une autre partie du code peut utiliser l'instance de classe pour verrouiller, et pourrait empêcher votre code à partir de l'obtention en temps opportun d'une serrure ou pourrait créer d'autres thread des problèmes de synchronisation. Dans le meilleur des cas: rien d'autre utilise une référence à votre classe de verrouillage. Carrure: quelque chose utilise une référence à votre classe pour faire des verrous et il provoque des problèmes de performances. Le pire des cas: quelque chose utilise une référence de votre classe pour faire des verrous et il provoque vraiment mauvais, vraiment subtil, vraiment difficile à déboguer des problèmes.

Veuillez consulter le lien suivant qui explique pourquoi de verrouillage (ce) n'est pas une bonne idée.

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

Donc la solution est d'ajouter un objet privé, par exemple, la zone lockobject à la classe et le lieu de la région de code à l'intérieur de l'instruction verrouillage comme indiqué ci-dessous:

lock (lockObject)
{
...
}

Vous pouvez établir une règle qui dit qu'une classe peut avoir des code que les verrous sur les " il " ou tout objet dont le code dans la classe instancie. Donc, c'est seulement un problème si le motif n'est pas suivie.

Si vous voulez vous protéger de code qui ne suivent pas ce modèle, l'on a accepté la réponse est correcte. Mais si la tendance est suivie, il n'est pas un problème.

L'avantage de verrouillage(ce) est l'efficacité. Que faire si vous avez un simple "objet de valeur" qui détient une valeur unique. C'est juste un wrapper, et il est instanciée des millions de fois. En exigeant la création d'un privé de synchronisation objet juste pour le verrouillage, vous avez doublé la taille de l'objet et a doublé le nombre d'allocations. Lorsque la performance est importante, c'est un avantage.

Lorsque vous n'avez pas de soins sur le nombre d'allocations ou de la mémoire, en évitant de verrouillage(ce) est préférable pour les raisons indiquées dans d'autres réponses.