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

Tut mir leid, Leute, aber ich kann dem Argument nicht zustimmen, dass das Sperren dieses zu einem Deadlock führen könnte. Ihr verwechselt zwei Dinge: Deadlocks und Verhungern.

• Sie können einen Deadlock nicht beenden, ohne einen der Threads zu unterbrechen, daher können Sie nach einem Deadlock nicht herauskommen
• Verhungern endet automatisch, nachdem einer der Threads seine Arbeit beendet hat

Hier ist ein Bild, das den Unterschied verdeutlicht.

Schlussfolgerung
Sie können immer noch sicher lock(this) verwenden, wenn Thread-Verhungern für Sie kein Problem ist. Sie müssen jedoch bedenken, dass wenn der Thread, der den Thread mit lock(this) verhungert, endet und das Objekt gesperrt ist, er schließlich in ewiger Verhungern endet ;)

Answer 2

Hier ist warum es nicht empfohlen wird.

Angenommen, Sie haben eine Klasse (SomeClass in diesem Beispiel) geschrieben und der Verbraucher Ihrer Klasse (ein Entwickler namens "John") möchte einen Sperrmodus über eine Instanz Ihrer Klasse (someObject in diesem Beispiel) erwerben. Er sperrt die Instanz someObject und ruft innerhalb dieses Sperrmodus eine Methode dieser Instanz auf (SomeMethod()), die intern einen Sperrmodus über die genau gleiche Instanz erwirbt.
Bisher so gut. Nun, stellen Sie sich vor, dass John es durch eine Task<> aufrufen möchte, Sie sehen einen Deadlock aufgrund dessen, dass das o-Objekt von einem anderen Prozess erworben wird, während die Methode SomeMethod() die genau gleiche Instanz in einem anderen Prozess erwerben muss.

• Obwohl John eine schlechte Praxis angewendet hat, indem er eine Instanz einer Klasse als Sperr-Objekt verwendet hat, sollten wir (als Entwickler einer Klassenbibliothek SomeClass) eine solche Situation einfach vermeiden, indem wir this nicht als Sperr-Objekt in unserer Klasse verwenden.

• Stattdessen sollten wir ein einfaches privates Feld deklarieren und das als unser Sperr-Objekt verwenden.

   using System;
   using System.Threading;
   using System.Threading.Tasks;
  
   class SomeClass
   {
       public void SomeMethod()
       {
           //HINWEIS: Sperrt über ein Objekt, das bereits vom Aufrufer gesperrt ist.
           //       Daher wird der folgende Code-Block nie ausgeführt.
           lock (this)
           {
               Console.WriteLine("Hi");
           }
       }
   }
  
   public class Program
   {
       public static void Main()
       {
           SomeClass o = new SomeClass();
  
           lock (o)
           {
               Task.Run(() => o.SomeMethod()).Wait();
           }
  
           Console.WriteLine("Fertig");
       }
   }