2009-04-08 15 views
52

Ich lese ein C# Buch, das das SyncRoot-Muster beschreibt. Es zeigtWas nutzt das SyncRoot-Muster?

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

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

und ist vergleichbar mit dem SyncRoot Muster:

object syncRoot = new object(); 

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

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

aber ich weiß nicht wirklich verstehen den Unterschied hier; In beiden Fällen scheint es möglich zu sein, dass beide Methoden jeweils nur für einen Thread zugänglich sind.

Das Buch beschreibt ... weil das Objekt der Instanz kann auch für den synchronisierten Zugriff von außen verwendet werden und Sie können diese Form nicht die Klasse selbst steuern, können Sie das SyncRoot-Muster verwenden Eh? "Gegenstand der Instanz"?

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

Vielen Dank im Voraus

+0

Im zweiten Beispiel ist SyncRoot() soll doThat sein()? –

+7

"Objekt der Instanz" == "Instanz der Klasse" Schlecht geschriebenes Buch. –

+0

Danke für die Bearbeitung Will Dean. Verdammt Kopieren/Einfügen :) – Ryan

Antwort

66

Wenn Sie eine interne Datenstruktur, die Sie gleichzeitig Zugriff auf von mehreren Threads verhindern wollen, sollten Sie immer sicherstellen, dass das Objekt, das Sie auf sind Sperren nicht öffentlich ist.

Die Begründung dahinter ist, dass ein öffentliches Objekt von jedem gesperrt werden kann, und damit können Sie Deadlocks erstellen, weil Sie nicht die vollständige Kontrolle über das Sperrmuster haben.

Das bedeutet, dass das Sperren auf this keine Option ist, da jeder dieses Objekt sperren kann. Genauso sollten Sie sich nicht an etwas festhalten, das Sie der Außenwelt aussetzen.

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

Das Sperren von Datenstrukturen ist etwas, das Sie wirklich beherrschen müssen, sonst riskieren Sie, ein Szenario für das Deadlocking einzurichten, was sehr problematisch sein kann. Hier

+13

OK. Wo kommt 'SyncRoot' ins Spiel? Warum haben die .NET-FCL-Designer '.SyncRoot' erstellt? Oder, um Ryans Frage zu paraphrasieren: * "Was nutzt das SyncRoot-Muster?" * –

+0

Wenn das schlimm ist, dann warum haben CLR-Designer * lock * keywoard an erster Stelle erstellt? Warum haben sie nicht die Klasse Lock erstellt, deren Instanz Sie in das Feld _syncRoot legen können? Und das würde locking Strukturen Overhead von jedem Object-Header entfernen –

+0

@IanBoyd - Die .SyncRoot gibt Entwicklern eine explizite Auswahl, wann immer sie die Auflistung (entweder selbst oder seine SyncRoot) sperren, um anzugeben, ob sie mit Sperren innerhalb des Codes interagieren möchten das implementiert die Sammlung oder wird garantiert nicht mit dem interagieren. Dies betrifft vor allem Code, der Sammlungen implementiert, die andere Sammlungen umhüllen (Vererbung, Dekoratoren, Verbundwerkstoffe ...). Nicht, ob dies gut oder schlecht ist. Zu sagen, dass die meisten Benutzer von Sammlungen sich nicht darum kümmern müssen. –

17

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 des ersten in einer lock-Anweisung verwenden kann. Dies führt im Beispiel zu einem Deadlock.

Die richtige SyncRoot Implementierung ist:

object syncRoot = new object(); 

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

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

als syncRoot ein privates Feld ist, müssen Sie sich keine Gedanken über die äußere Anwendung dieses Objekts kümmern.

+5

Dies wird nicht wirklich fehlschlagen, da sie auf dem gleichen Thread laufen ... –

+1

IMHO ist dies eine einfache Sperre, die nicht freigegeben wurde. nichts mehr. Das ist kein totes Schloss. wo ein Thread auf einen anderen wartet. –

+0

Nun, außer die Sperre wird freigegeben (nachdem beide Lock-Anweisungen beendet sind) - dies wird nicht fehlschlagen. Definitiv keine Sackgasse. – BrainSlugs83

6

Siehe this Jeff Richters Artikel. Insbesondere in diesem Beispiel, das dass ein Deadlock auf „diese“ Verriegelung zeigt verursachen können:

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... 
     } 
    } 
} 
+0

Nur-Link-Antwort. Beispiel, das zeigt, dass bösartiger Code so konstruiert werden kann, dass ein Deadlock auftritt. Und die Verbindung baumelt. –

11

Hier ist eine andere interessante Sache zu diesem Thema:

Questionable value of SyncRoot on Collections (by Brad Adams):

Sie über bemerken Sie eine SyncRoot-Eigenschaft auf viele der Sammlungen in System.Collections.Rückblickend denke ich , dass diese Eigenschaft ein Fehler war. Krzysztof Cwalina, ein Programm Manger auf mein Team hat mir gerade ein paar Gedanken auf warum das so ist - ich stimme ihm zu:

Wir fanden die SyncRoot-basierte Synchronisation APIs nicht flexibel genug für die meisten Szenarien zu sein . Die APIs ermöglichen Gewinde sicheren Zugang zu einem einzelnen Mitglied einer Sammlung . Das Problem ist, dass es gibt zahlreiche Szenarien, in denen Sie müssen auf mehrere Operationen sperren (für Beispiel ein Element entfernen und eine weitere hinzufügen). Mit anderen Worten, es ist in der Regel der Code, der eine Sammlung verwendet, dass wählen will (und tatsächlich implementieren kann), um die richtige Synchronisation Politik, nicht die Sammlung selbst. Wir festgestellt, dass SyncRoot tatsächlich sehr selten verwendet wird und in Fällen, in denen es verwendet wird, fügt es tatsächlich nicht viel Wert hinzu. In Fällen, in denen es nicht verwendet wird, es ist nur ein Ärgernis zu Implementierer von ICollection.

Seien Sie versichert, wir werden nicht den gleichen Fehler machen, wie wir die generische Versionen dieser Sammlungen

+2

Das SyncRoot auf Sammlungen ist nicht dasselbe, über das dieses Thema spricht, wodurch das private Feld SyncRoot public so schlecht ist wie das Sperren auf "this". Das Thema vergleicht das Sperren auf private readonly-Felder mit dem Sperren von "this". Im Fall von Collections wurde das private Feld über eine öffentliche Eigenschaft zugänglich gemacht. Dies ist keine bewährte Methode und kann zu Deadlocks führen. – haze4real

+0

@Haze Können Sie erklären, dass der Teil "durch eine öffentliche Eigenschaft zugänglich gemacht wurde, was keine Best Practice ist und zu Deadlocks führen kann". Wie kann hier ein Deadlock passieren? – prabhakaran

+0

@prabhakaran Das Problem bei der Sperrung von 'this', einem öffentlichen Feld oder einem privaten Feld, das über eine öffentliche Eigenschaft verfügbar gemacht wird, ist, dass jeder darauf zugreifen kann, was zu Deadlocks führen kann, wenn Sie die Implementierung nicht kennen. Klassen sollten nicht so gestaltet sein, dass die Kenntnis der Implementierung der Klasse für ihre korrekte Verwendung erforderlich ist. – haze4real

1

Ein weiteres konkretes Beispiel bauen:

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 wird der erste Anruf wird gelingen , aber wenn Sie im Debugger verfolgen, blockiert der Aufruf von DoSomething, bis die Sperre freigegeben wird. Der zweite Anruf wird Deadlock, da der Hauptfaden der Bildschirmsperre auf Test halten.

Das Problem ist, dass Main die Objektinstanz sperren kann, was bedeutet, dass es die Instanz davon abhalten kann, alles zu tun, was das Objekt für synchronisiert hält. Der Punkt ist, dass das Objekt selbst weiß, was gesperrt werden muss, und externe Störungen verlangen nur nach Ärger. Deshalb ist das Muster, das eine private Variable zu haben, die Sie ausschließlich zur Synchronisation, ohne sich um Einmischung von außen sorgen können.

Das gleiche gilt für die ä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); 
    } 
} 

ein eigenes statisches Objekt Verwenden Sie auf synchronisieren, nicht der Typ.

9

Der eigentliche Zweck dieses Musters ist die Implementierung der korrekten Synchronisation mit der Wrapper-Hierarchie.

Zum Beispiel, wenn Klasse WrapperA eine Instanz von ClassThanNeedsToBeSynced Wraps, und Klasse WrapperB wickelt die gleiche Instanz von ClassThanNeedsToBeSynced, können Sie nicht sperren auf WrapperA oder WrapperB, da, wenn Sie auf WrapperA sperren, Sperre auf WrappedB nicht warten . Aus diesem Grund müssen Sie wrapperAInst.SyncRoot und wrapperBInst.SyncRoot sperren, die die Sperre an die Klasse 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 
     } 
    } 
} 
+0

Vielen Dank für die Beantwortung der Frage! – Govert