Warum ist lock(this) {...} schlecht?

Question

Die MSDN-Dokumentation besagt, dass

public class SomeObject
{
  public void SomeOperation()
  {
    lock(this)
    {
      //Zugriff auf Instanzvariablen
    }
  }
}

ein "Problem darstellt, wenn die Instanz öffentlich zugänglich ist". Ich frage mich warum? Liegt es daran, dass das Schloss länger als nötig gehalten wird? Oder gibt es einen noch hinterhältigeren Grund?

Answer 1

Stell dir vor, du hast eine erfahrene Sekretärin in deinem Büro, die eine gemeinsam genutzte Ressource in der Abteilung ist. Ab und zu stürmst du auf sie zu, weil du eine Aufgabe hast, nur um zu hoffen, dass sich nicht schon einer deiner Kollegen ihrer bedient hat. Normalerweise musst du nur eine kurze Zeit warten.

Weil Fürsorge gleichbedeutend ist mit Teilen, entscheidet dein Manager, dass Kunden die Sekretärin auch direkt nutzen können. Aber das hat eine Nebenwirkung: Ein Kunde könnte sie sogar beanspruchen, während du gerade für diesen Kunden arbeitest und sie auch benötigst, um einen Teil der Aufgaben auszuführen. Es kommt zu einer Sackgasse, weil die Beanspruchung hierarchisch nicht mehr geordnet ist. Dies hätte von Anfang an vermieden werden können, indem man Kunden nicht erlaubt hätte, sie zu beanspruchen.

lock(this) ist schlecht, wie wir gesehen haben. Ein externes Objekt könnte sich auf das Objekt sperren und da du nicht kontrollierst, wer die Klasse nutzt, kann sich jeder darauf sperren... Genau wie im obigen Beispiel beschrieben. Auch hier ist die Lösung, die Exposition des Objekts zu begrenzen. Wenn du jedoch eine private, protected oder internal Klasse hast, könntest du bereits kontrollieren, wer sich auf dein Objekt sperrt, weil du sicher bist, dass du deinen Code selbst geschrieben hast. Die Botschaft hier lautet also: stelle es nicht als public zur Verfügung. Auch die Verwendung eines Sperrmechanismus in ähnlichen Szenarien vermeidet Sackgassen.

Das genaue Gegenteil davon ist das Sperren von Ressourcen, die im gesamten Anwendungsdomäne gemeinsam genutzt werden - der schlimmste Fall. Das wäre so, als ob du deine Sekretärin nach draußen stellst und es jedem erlaubst, sie zu beanspruchen. Das Ergebnis ist völliges Chaos - oder in Bezug auf den Quellcode: es war eine schlechte Idee; wirf sie weg und fange von vorne an. Wie machen wir das also?

Typen werden in der Anwendungsdomäne geteilt, wie die meisten hier betonen. Aber es gibt sogar bessere Dinge, die wir nutzen können: Strings. Der Grund dafür ist, dass Strings gepoolt sind. Mit anderen Worten: Wenn du in einer Anwendungsdomäne zwei Strings hast, die den gleichen Inhalt haben, besteht die Möglichkeit, dass sie den genau gleichen Zeiger haben. Da der Zeiger als Sperrschlüssel verwendet wird, bekommst du im Grunde genommen ein Synonym für "Bereite dich auf undefiniertes Verhalten vor".

Ähnlich solltest du dich nicht auf WCF-Objekte, HttpContext.Current, Thread.Current, Singletons (allgemein) usw. sperren. Der einfachste Weg, all dem auszuweichen? private [static] object myLock = new object();

Answer 2

Das Sperren des dieses-Zeigers kann schlecht sein, wenn Sie über eine gemeinsame Ressource sperren. Eine gemeinsame Ressource kann eine statische Variable oder eine Datei auf Ihrem Computer sein - d.h. etwas, das zwischen allen Benutzern der Klasse gemeinsam genutzt wird. Der Grund dafür ist, dass der this-Zeiger jedes Mal, wenn Ihre Klasse instanziiert wird, eine andere Referenz zu einem Speicherort im Speicher enthält. Das Sperren über dieses in einer Instanz einer Klasse unterscheidet sich also von dem Sperren über dieses in einer anderen Instanz einer Klasse.

Werfen Sie einen Blick auf diesen Code, um zu sehen, was ich meine. Fügen Sie den folgenden Code Ihrem Hauptprogramm in einer Konsolenanwendung hinzu:

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

Erstellen Sie eine neue Klasse wie unten.

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

    public void Withdraw(int amount)
    {
      // Probieren Sie beide Sperren aus, um zu sehen, was ich meine
      //             lock (this)
       lock (myLock)
        {
            Random rand = new Random();
            if (balance >= amount)
            {
                Console.WriteLine("Kontostand vor Abhebung:  " + balance);
                Console.WriteLine("Abheben        : -" + amount);
                balance = balance - amount;
                Console.WriteLine("Kontostand nach Abhebung  :  " + balance);
            }
            else
            {
                Console.WriteLine("Ihre Transaktion kann nicht durchgeführt werden, da der aktuelle Kontostand:  " + balance + " ist und Sie versucht haben, " + amount + " abzuheben");
            }
        }

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

Hier ist eine Ausführung des Programms, bei der das dieses gesperrt ist.

   Kontostand vor Abhebung:  100000
    Abheben        : -5600
    Kontostand nach Abhebung  :  94400
    Kontostand vor Abhebung:  100000
    Kontostand vor Abhebung:  100000
    Abheben        : -5600
    Kontostand nach Abhebung  :  88800
    Abheben        : -5600
    Kontostand nach Abhebung  :  83200
    Kontostand vor Abhebung:  83200
    Abheben        : -9100
    Kontostand nach Abhebung  :  74100
    Kontostand vor Abhebung:  74100
    Abheben        : -9100
    Kontostand vor Abhebung:  74100
    Abheben        : -9100
    Kontostand nach Abhebung  :  55900
    Kontostand nach Abhebung  :  65000
    Kontostand vor Abhebung:  55900
    Abheben        : -9100
    Kontostand nach Abhebung  :  46800
    Kontostand vor Abhebung:  46800
    Abheben        : -2800
    Kontostand nach Abhebung  :  44000
    Kontostand vor Abhebung:  44000
    Abheben        : -2800
    Kontostand nach Abhebung  :  41200
    Kontostand vor Abhebung:  44000
    Abheben        : -2800
    Kontostand nach Abhebung  :  38400

Hier ist eine Ausführung des Programms, bei dem auf myLock gesperrt wird.

Kontostand vor Abhebung:  100000
Abheben        : -6600
Kontostand nach Abhebung  :  93400
Kontostand vor Abhebung:  93400
Abheben        : -6600
Kontostand nach Abhebung  :  86800
Kontostand vor Abhebung:  86800
Abheben        : -200
Kontostand nach Abhebung  :  86600
Kontostand vor Abhebung:  86600
Abheben        : -8500
Kontostand nach Abhebung  :  78100
Kontostand vor Abhebung:  78100
Abheben        : -8500
Kontostand nach Abhebung  :  69600
Kontostand vor Abhebung:  69600
Abheben        : -8500
Kontostand nach Abhebung  :  61100
Kontostand vor Abhebung:  61100
Abheben        : -2200
Kontostand nach Abhebung  :  58900
Kontostand vor Abhebung:  58900
Abheben        : -2200
Kontostand nach Abhebung  :  56700
Kontostand vor Abhebung:  56700
Abheben        : -2200
Kontostand nach Abhebung  :  54500
Kontostand vor Abhebung:  54500
Abheben        : -500
Kontostand nach Abhebung  :  54000

Answer 3

Es gibt einen sehr guten Artikel darüber http://bytes.com/topic/c-sharp/answers/249277-dont-lock-type-objects von Rico Mariani, Leistungsarchitekt für die Microsoft® .NET-Laufzeitumgebung

Auszug:

Das grundlegende Problem hier ist, dass Sie das Typobjekt nicht besitzen und nicht wissen, wer sonst darauf zugreifen könnte. Im Allgemeinen ist es eine sehr schlechte Idee, sich darauf zu verlassen, ein Objekt zu sperren, das Sie nicht erstellt haben und nicht wissen, wer sonst darauf zugreifen könnte. Dies führt zu einem Deadlock. Der sicherste Weg ist es, nur private Objekte zu sperren.

Answer 4

Es gibt auch eine gute Diskussion dazu hier: Ist dies der richtige Einsatz eines Mutex?

Answer 5

Weil jeder Codeblock, der die Instanz Ihrer Klasse sehen kann, auch auf diese Referenz sperren kann. Sie möchten Ihr Sperrobjekt verstecken (kapseln), so dass nur der Code, der darauf verweisen muss, darauf verweisen kann. Das Schlüsselwort this bezieht sich auf die aktuelle Klasseninstanz, so dass beliebig viele Dinge darauf verweisen könnten und es zur Thread-Synchronisierung verwenden könnten.

Um es klar auszudrücken, dies ist schlecht, weil ein anderer Codeblock die Klasseninstanz zum Sperren verwenden könnte und möglicherweise verhindert, dass Ihr Code rechtzeitig gesperrt wird oder andere Thread-Synchronisierungsprobleme verursachen könnte. Im besten Fall: Nichts anderes verwendet einen Verweis auf Ihre Klasse zum Sperren. Mittelfall: Etwas verwendet einen Verweis auf Ihre Klasse, um Sperren durchzuführen, und es verursacht Leistungsprobleme. Schlimmster Fall: Etwas verwendet einen Verweis auf Ihre Klasse, um Sperren durchzuführen, und es verursacht wirklich schlechte, wirklich subtile, wirklich schwer zu debuggende Probleme.