Was nützt das SyncRoot-Muster?

68

Ich lese ein Buch, das das SyncRoot-Muster beschreibt. Es zeigt

void doThis()
{
    lock(this){ ... }
}

void doThat()
{
    lock(this){ ... }
}

und vergleicht mit dem SyncRoot-Muster:

object syncRoot = new object();

void doThis()
{
    lock(syncRoot ){ ... }
}

void doThat()
{
    lock(syncRoot){ ... }
}

Allerdings verstehe ich den Unterschied hier nicht wirklich; Es scheint, dass in beiden Fällen jeweils nur ein Thread auf beide Methoden zugreifen kann.

Das Buch beschreibt ... da das Objekt der Instanz auch für den synchronisierten Zugriff von außen verwendet werden kann und Sie dies nicht von der Klasse selbst aus steuern können, können Sie das SyncRoot-Muster verwenden. Eh? 'Objekt der Instanz'?

Kann mir jemand den Unterschied zwischen den beiden oben genannten Ansätzen erklären?

Ryan
quelle
Soll syncRoot () im zweiten Beispiel doThat () sein?
Will Dean
9
"Objekt der Instanz" == "Instanz der Klasse" Buch mit schlechten Worten.
Darren Clark
Danke für die Bearbeitung Will Dean. Verdammtes Kopieren / Einfügen :)
Ryan

Antworten:

75

Wenn Sie über eine interne Datenstruktur verfügen, auf die der gleichzeitige Zugriff mehrerer Threads verhindern soll, sollten Sie immer sicherstellen, dass das Objekt, für das Sie sich sperren, nicht öffentlich ist.

Der Grund dafür ist, dass ein öffentliches Objekt von jedem gesperrt werden kann und Sie somit Deadlocks erstellen können, da Sie nicht die vollständige Kontrolle über das Sperrmuster haben.

Dies bedeutet, dass das Sperren thisnicht möglich ist, da jeder dieses Objekt sperren kann. Ebenso sollten Sie sich nicht auf etwas festlegen, das Sie der Außenwelt aussetzen.

Was bedeutet, dass die beste Lösung darin besteht, ein internes Objekt zu verwenden, und daher ist der Tipp, es nur zu verwenden Object.

Das Sperren von Datenstrukturen ist etwas, über das Sie wirklich die volle Kontrolle haben müssen. Andernfalls besteht die Gefahr, dass Sie ein Szenario für das Deadlocking einrichten, dessen Handhabung sehr problematisch sein kann.

Lasse V. Karlsen
quelle
16
IN ORDNUNG. Wo kommt das SyncRootrein? Warum haben die .NET FCL-Designer erstellt .SyncRoot? Oder um Ryans Frage zu paraphrasieren: "Was nützt das SyncRoot-Muster?"
Ian Boyd
Wenn das schlecht ist, warum haben CLR-Designer dann überhaupt den Lock Keywoard erstellt ? Warum haben sie keine Klassensperre erstellt, deren Instanz Sie in das Feld _syncRoot einfügen können? Und das würde Sperrstrukturen über Kopf aus dem Header jedes Objekts entfernen
Bogdan Mart
@IanBoyd - Mit .SyncRoot können Entwickler explizit auswählen, wann immer sie die Sammlung sperren (entweder selbst oder deren SyncRoot), um anzugeben, ob sie mit Sperren innerhalb des Codes interagieren möchten, der die Sammlung implementiert, oder ob sie garantiert nicht interagieren Das. Dies betrifft hauptsächlich Code-Implementierungssammlungen, die andere Sammlungen (Vererbung, Dekoratoren, Verbundwerkstoffe ...) umschließen. Sie sagen nicht, ob dies gut oder schlecht ist. Zu sagen, dass die meisten Benutzer von Sammlungen sich nicht darum kümmern müssen.
Jirka Hanika
18

Hier ist ein Beispiel :

class ILockMySelf
{
    public void doThat()
    {
        lock (this)
        {
            // Don't actually need anything here.
            // In this example this will never be reached.
        }
    }
}

class WeveGotAProblem
{
    ILockMySelf anObjectIShouldntUseToLock = new ILockMySelf();

    public void doThis()
    {
        lock (anObjectIShouldntUseToLock)
        {
            // doThat will wait for the lock to be released to finish the thread
            var thread = new Thread(x => anObjectIShouldntUseToLock.doThat());
            thread.Start();

            // doThis will wait for the thread to finish to release the lock
            thread.Join();
        }
    }
}

Sie sehen, dass die zweite Klasse eine Instanz der ersten in einer lock-Anweisung verwenden kann. Dies führt zu einem Deadlock im Beispiel.

Die korrekte SyncRoot-Implementierung lautet:

object syncRoot = new object();

void doThis()
{
    lock(syncRoot ){ ... }
}

void doThat()
{
    lock(syncRoot ){ ... }
}

Da syncRootes sich um ein privates Feld handelt, müssen Sie sich nicht um die externe Verwendung dieses Objekts kümmern.

ybo
quelle
15

Der eigentliche Zweck dieses Musters besteht darin, eine korrekte Synchronisation mit der Wrapper-Hierarchie zu implementieren.

Wenn beispielsweise die Klasse WrapperA eine Instanz von ClassThanNeedsToBeSynced umschließt und die Klasse WrapperB dieselbe Instanz von ClassThanNeedsToBeSynced umschließt, können Sie WrapperA oder WrapperB nicht sperren, da die Sperre von WrapperB nicht wartet, wenn Sie WrapperA sperren. Aus diesem Grund müssen Sie wrapperAInst.SyncRoot und wrapperBInst.SyncRoot sperren, die die Sperre an ClassThanNeedsToBeSynced delegieren.

Beispiel:

public interface ISynchronized
{
    object SyncRoot { get; }
}

public class SynchronizationCriticalClass : ISynchronized
{
    public object SyncRoot
    {
        // you can return this, because this class wraps nothing.
        get { return this; }
    }
}

public class WrapperA : ISynchronized
{
    ISynchronized subClass;

    public WrapperA(ISynchronized subClass)
    {
        this.subClass = subClass;
    }

    public object SyncRoot
    {
        // you should return SyncRoot of underlying class.
        get { return subClass.SyncRoot; }
    }
}

public class WrapperB : ISynchronized
{
    ISynchronized subClass;

    public WrapperB(ISynchronized subClass)
    {
        this.subClass = subClass;
    }

    public object SyncRoot
    {
        // you should return SyncRoot of underlying class.
        get { return subClass.SyncRoot; }
    }
}

// Run
class MainClass
{
    delegate void DoSomethingAsyncDelegate(ISynchronized obj);

    public static void Main(string[] args)
    {
        SynchronizationCriticalClass rootClass = new SynchronizationCriticalClass();
        WrapperA wrapperA = new WrapperA(rootClass);
        WrapperB wrapperB = new WrapperB(rootClass);

        // Do some async work with them to test synchronization.

        //Works good.
        DoSomethingAsyncDelegate work = new DoSomethingAsyncDelegate(DoSomethingAsyncCorrectly);
        work.BeginInvoke(wrapperA, null, null);
        work.BeginInvoke(wrapperB, null, null);

        // Works wrong.
        work = new DoSomethingAsyncDelegate(DoSomethingAsyncIncorrectly);
        work.BeginInvoke(wrapperA, null, null);
        work.BeginInvoke(wrapperB, null, null);
    }

    static void DoSomethingAsyncCorrectly(ISynchronized obj)
    {
        lock (obj.SyncRoot)
        {
            // Do something with obj
        }
    }

    // This works wrong! obj is locked but not the underlaying object!
    static void DoSomethingAsyncIncorrectly(ISynchronized obj)
    {
        lock (obj)
        {
            // Do something with obj
        }
    }
}
Roman Zavalov
quelle
2
Vielen Dank, dass Sie die Frage tatsächlich beantwortet haben!
Govert
14

Hier ist noch eine interessante Sache zu diesem Thema:

Fragwürdiger Wert von SyncRoot für Sammlungen (von Brad Adams) :

Sie werden eine SyncRootEigenschaft auf vielen der Sammlungen in bemerken System.Collections. Im Nachhinein denke ich, dass diese Eigenschaft ein Fehler war. Krzysztof Cwalina, ein Programmmanager in meinem Team, hat mir nur einige Gedanken darüber geschickt, warum das so ist - ich stimme ihm zu:

Wir haben festgestellt, dass die auf der SyncRootSynchronisation basierenden Synchronisations-APIs für die meisten Szenarien nicht ausreichend flexibel sind. Die APIs ermöglichen einen thread-sicheren Zugriff auf ein einzelnes Mitglied einer Sammlung. Das Problem ist, dass es zahlreiche Szenarien gibt, in denen Sie mehrere Vorgänge sperren müssen (z. B. ein Element entfernen und ein anderes hinzufügen). Mit anderen Worten, es ist normalerweise der Code, der eine Sammlung verwendet, die die richtige Synchronisationsrichtlinie auswählen möchte (und tatsächlich implementieren kann), nicht die Sammlung selbst. Wir haben festgestellt, dass dies SyncRoottatsächlich sehr selten verwendet wird und in Fällen, in denen es verwendet wird, tatsächlich nicht viel Mehrwert bringt. In Fällen, in denen es nicht verwendet wird, ist es nur ein Ärger für Implementierer von ICollection.

Seien Sie versichert, dass wir nicht den gleichen Fehler machen werden, wie wir die generischen Versionen dieser Sammlungen erstellen.

Igor Brejc
quelle
3
Der SyncRoot für Sammlungen ist nicht derselbe, von dem in diesem Thema die Rede ist. Das private Feld SyncRoot öffentlich zu machen ist genauso schlecht wie das Sperren von "this". Das Thema besteht darin, das Sperren für private schreibgeschützte Felder mit dem Sperren für "dies" zu vergleichen. Im Fall von Sammlungen wurde das private Feld über eine öffentliche Eigenschaft zugänglich gemacht, was nicht die beste Vorgehensweise ist und zu Deadlocks führen kann.
Haze4real
@Haze Kannst du erklären, dass der Teil "über ein öffentliches Eigentum zugänglich gemacht wurde, was nicht die beste Vorgehensweise ist und zu Deadlocks führen kann". Wie kann hier ein Deadlock passieren?
Prabhakaran
@prabhakaran Das Problem beim Sperren thiseines öffentlichen oder privaten Felds, das über eine öffentliche Eigenschaft verfügbar gemacht wird, besteht darin, dass jeder darauf zugreifen kann. Dies kann zu Deadlocks führen, wenn Sie sich der Implementierung nicht bewusst sind. Klassen sollten nicht so gestaltet sein, dass das Wissen über die Implementierung der Klasse für ihre korrekte Verwendung erforderlich ist.
Haze4real
@prabhakaran Ein konkretes Beispiel finden Sie in Darren Clarks Antwort.
Haze4real
6

Siehe diesen Artikel von Jeff Richter. Insbesondere dieses Beispiel, das zeigt, dass das Sperren von "this" einen Deadlock verursachen kann:

using System;
using System.Threading;

class App {
   static void Main() {
      // Construct an instance of the App object
      App a = new App();

      // This malicious code enters a lock on 
      // the object but never exits the lock
      Monitor.Enter(a);

      // For demonstration purposes, let's release the 
      // root to this object and force a garbage collection
      a = null;
      GC.Collect();

      // For demonstration purposes, wait until all Finalize
      // methods have completed their execution - deadlock!
      GC.WaitForPendingFinalizers();

      // We never get to the line of code below!
      Console.WriteLine("Leaving Main");
   }

   // This is the App type's Finalize method
   ~App() {
      // For demonstration purposes, have the CLR's 
      // Finalizer thread attempt to lock the object.
      // NOTE: Since the Main thread owns the lock, 
      // the Finalizer thread is deadlocked!
      lock (this) {
         // Pretend to do something in here...
      }
   }
}
Anton Gogolev
quelle
Nur-Link-Antwort. Beispiel, das zeigt, dass bösartiger Code so konstruiert werden kann, dass er blockiert. Und die Verbindung baumelt.
Jirka Hanika
2

Ein weiteres konkretes Beispiel:

class Program
{
    public class Test
    {
        public string DoThis()
        {
            lock (this)
            {
                return "got it!";
            }
        }
    }

    public delegate string Something();

    static void Main(string[] args)
    {
        var test = new Test();
        Something call = test.DoThis;
        //Holding lock from _outside_ the class
        IAsyncResult async;
        lock (test)
        {
            //Calling method on another thread.
            async = call.BeginInvoke(null, null);
        }
        async.AsyncWaitHandle.WaitOne();
        string result = call.EndInvoke(async);

        lock (test)
        {
            async = call.BeginInvoke(null, null);
            async.AsyncWaitHandle.WaitOne();
        }
        result = call.EndInvoke(async);
    }
}

In diesem Beispiel ist der erste Aufruf erfolgreich. Wenn Sie jedoch im Debugger nachverfolgen, wird der Aufruf von DoSomething blockiert, bis die Sperre aufgehoben wird. Der zweite Aufruf wird blockiert, da der Haupt-Thread die Monitorsperre beim Test hält .

Das Problem ist, dass Main die Objektinstanz sperren kann, was bedeutet, dass die Instanz alles verhindern kann, was das Objekt für synchronisiert hält. Der Punkt ist, dass das Objekt selbst weiß, was gesperrt werden muss, und dass Störungen von außen nur nach Problemen fragen. Aus diesem Grund das Muster einer privaten Mitgliedsvariablen, die Sie ausschließlich für die Synchronisierung verwenden können, ohne sich um Störungen von außen sorgen zu müssen.

Gleiches gilt für das äquivalente statische Muster:

class Program
{
    public static class Test
    {
        public static string DoThis()
        {
            lock (typeof(Test))
            {
                return "got it!";
            }
        }
    }

    public delegate string Something();

    static void Main(string[] args)
    {
        Something call =Test.DoThis;
        //Holding lock from _outside_ the class
        IAsyncResult async;
        lock (typeof(Test))
        {
            //Calling method on another thread.
            async = call.BeginInvoke(null, null);
        }
        async.AsyncWaitHandle.WaitOne();
        string result = call.EndInvoke(async);

        lock (typeof(Test))
        {
            async = call.BeginInvoke(null, null);
            async.AsyncWaitHandle.WaitOne();
        }
        result = call.EndInvoke(async);
    }
}

Verwenden Sie zum Synchronisieren ein privates statisches Objekt, nicht den Typ.

Darren Clark
quelle