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

Question

La documentation MSDN dit que

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

est "un problème si l'instance est accessible au public". Je me demande pourquoi? Est-ce parce que la serrure se tiendra plus longtemps que nécessaire? Ou est-il une plus insidieux raison?

Answer 1

C'est mauvais d'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 bien planifier 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 l' 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 y a l'idée répandue qu' 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 à l' lock sert seulement à la 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 il est mauvais d'utiliser les chaînes de caractères comme les touches en lock des déclarations, étant donné qu'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, une Object exemple fera très bien l'affaire.

Exécuter le code C# suivant comme un 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'.

Answer 2

Car si les gens peuvent obtenir auprès de votre instance de l'objet (c'est à dire: votre this) pointeur, alors ils peuvent également essayer de verrouiller ce même objet. Maintenant, ils peuvent ne pas être conscient que vous êtes le verrouillage this en interne, ce qui peut causer des problèmes (peut-être une impasse)

En plus de cela, c'est aussi une mauvaise pratique, car c'est le verrouillage "trop"

Par exemple, vous pourriez avoir une variable de membre de l' List<int>, et la seule chose que vous avez réellement besoin de verrou est que la variable de membre. Si vous verrouillez l'ensemble de l'objet dans vos fonctions, puis d'autres choses qui appeler ces fonctions seront bloqués en attente de la serrure. Si ces fonctions n'ont pas besoin d'accéder à la liste des membres, vous allez être à l'origine d'un autre code d'attendre et de ralentir votre application pour aucune raison du tout.

Answer 3

Jetez un oeil à la Rubrique MSDN de Synchronisation de Thread (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.

Answer 4

Je sais que c'est un vieux thread, mais parce que les gens peuvent encore faire des recherches et de compter sur elle, il semble important de souligner que, lock(typeof(SomeObject)) est nettement moins bonne que lock(this). Ayant dit cela; sincère bravo à Alan pour préciser que lock(typeof(SomeObject)) est une mauvaise pratique.

Une instance de System.Type est l'un des plus générique, à grain grossier, objets, il est. À tout le moins, une instance de Système.Le Type est global à un domaine d'application, et .NET peut exécuter plusieurs programmes dans un domaine d'application. Cela signifie que les deux programmes totalement différents pourrait potentiellement causer de l'interférence de l'un à l'autre, voire à la création d'un blocage si ils ont tous les deux essayer d'obtenir un verrou de synchronisation sur le même type d'instance.

Donc, lock(this) n'est pas particulièrement robuste, peut causer des problèmes et il faut toujours remonter les sourcils, pour toutes les raisons citées. Pourtant, il est largement utilisé, relativement bien respecté et apparemment stable, un code comme log4net qui utilise la serrure(ce) modèle largement, même si je préfère personnellement de voir que le changement de motif.

Mais lock(typeof(SomeObject)) ouvre une toute nouvelle et améliorée de la boîte de pandore.

Pour ce que ça vaut.

Answer 5

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

lock(typeof(SomeObject))