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

Sie können eine Regel aufstellen, die besagt, dass eine Klasse Code haben kann, der auf 'this' oder auf jedes Objekt sperrt, das der Code in der Klasse instanziiert. Es ist also nur ein Problem, wenn das Muster nicht befolgt wird.

Wenn Sie sich vor Code schützen möchten, der dieses Muster nicht befolgt, dann ist die akzeptierte Antwort korrekt. Aber wenn das Muster befolgt wird, gibt es kein Problem.

Der Vorteil von lock(this) liegt in der Effizienz. Was ist, wenn Sie ein einfaches "Wertobjekt" haben, das einen einzigen Wert enthält. Es ist nur ein Wrapper und wird Millionen von Malen instanziiert. Wenn Sie die Erstellung eines privaten Synchronisierungsobjekts nur für die Sperrung benötigen, haben Sie im Grunde die Größe des Objekts verdoppelt und die Anzahl der Zuweisungen verdoppelt. Wenn die Leistung wichtig ist, ist dies ein Vorteil.

Wenn Sie sich nicht um die Anzahl der Zuweisungen oder den Speicherbedarf kümmern, ist es aus den Gründen, die in anderen Antworten angegeben sind, vorzuziehen, lock(this) zu vermeiden.

Answer 2

Hier ist ein Beispielcode, dem meiner Meinung nach einfacher zu folgen ist (funktioniert in LinqPad, referenziere folgende Namespaces: System.Net und System.Threading.Tasks)

Etwas zu beachten ist, dass lock(x) im Grunde syntaktischer Zucker ist und was es tut, ist, Monitor.Enter zu verwenden und dann einen try, catch, finally-Block zu verwenden, um Monitor.Exit aufzurufen. Siehe: https://learn.microsoft.com/en-us/dotnet/api/system.threading.monitor.enter (Bemerkungen)

 

oder verwenden Sie die C#-lock-Anweisung (SyncLock-Anweisung in Visual Basic),   die die Enter- und Exit-Methoden in einem try…finally-Block einbettet.

Ausgabe

CurrentThread 15
CurrentThread 15
Start ClassTest.DoWorkUsingMonitor 2 CurrentThread 13
Start ClassTest.DoWorkUsingThisLock 1 CurrentThread 12
Skipped lock section!  2 CurrentThread 13
End ClassTest.DoWorkUsingMonitor Done 2 CurrentThread 13

Beachten Sie, dass Thread#12 nie endet, da er durch eine Deadlock blockiert ist.

Answer 3

Bitte beachten Sie den folgenden Link, der erklärt, warum lock (this) keine gute Idee ist.

https://learn.microsoft.com/en-us/dotnet/standard/threading/managed-threading-best-practices

Die Lösung besteht darin, ein privates Objekt, z. B. lockObject, der Klasse hinzuzufügen und den Codebereich innerhalb der lock-Anweisung wie unten gezeigt zu platzieren:

lock (lockObject)
{
...
}

Answer 4

Hier ist eine viel einfachere Illustration (entnommen aus Frage 34 hier), warum lock(this) schlecht ist und zu Deadlocks führen kann, wenn der Verbraucher Ihrer Klasse auch versucht, das Objekt zu sperren. Unten kann nur einer von drei Threads fortschreiten, die anderen beiden sind in einem Deadlock.

class SomeClass
{
    public void SomeMethod(int id)
    {
        **lock(this)**
        {
            while(true)
            {
                Console.WriteLine("SomeClass.SomeMethod #" + id);
            }
        }
    }
}

class Program
{
    static void Main(string[] args)
    {
        SomeClass o = new SomeClass();

        lock(o)
        {
            for (int threadId = 0; threadId < 3; threadId++)
            {
                Thread t = new Thread(() => {
                    o.SomeMethod(threadId);
                        });
                t.Start();
            }

            Console.WriteLine();
        }

Um dies zu umgehen, verwendete dieser Kerl Thread.TryMonitor (mit Timeout) anstelle von lock:

            Monitor.TryEnter(temp, millisecondsTimeout, ref lockWasTaken);
            if (lockWasTaken)
            {
                doAction();
            }
            else
            {
                throw new Exception("Konnte Sperre nicht erhalten");
            }

https://blogs.appbeat.io/post/c-how-to-lock-without-deadlocks

Answer 5

Es wird ein Problem geben, wenn die Instanz öffentlich zugänglich ist, da es andere Anfragen geben könnte, die dieselbe Objektinstanz verwenden könnten. Es ist besser, private/statische Variable zu verwenden.