Sperren für das Objekt, das synchronisiert wird oder ein dediziertes Sperrobjekt verwendet?

Question

So weit ich von Kollegen und dem Internet verstanden habe, ist es eine schlechte Übung, das Objekt zu synchronisieren, das synchronisiert wird, aber was ich nicht verstehe, ist warum?

Die folgende Klasse soll Einstellungen in ein Wörterbuch laden, und es gibt auch eine Methode zum Abrufen von Einstellungen.

public class TingbogSettingService : ITingbogSettingService 
{ 
    private readonly ISettingRepository _settingRepository; 
    private readonly ICentralLog _centralLog; 
    private Dictionary<string, ISetting> _settingDictionary = new Dictionary<string, ISetting>(); 


    public TingbogSettingService(ISettingRepository settingRepository, ICentralLog centralLog) 
    { 
     _settingRepository = settingRepository; 
     _centralLog = centralLog; 
    } 

    public ISetting GetSetting(string settingName) 
    { 
     ISetting setting; 
     if (!_settingDictionary.TryGetValue(settingName, out setting)) 
     { 
      return null; 
     } 
     return setting; 
    } 

    public void LoadSettings() 
    { 
     var settings = _settingRepository.LoadSettings(); 
     try 
     { 
      lock (_settingDictionary) 
      { 
       _settingDictionary = settings.ToDictionary(x => x.SettingName); 
      }     
     } 
     catch (Exception ex) 
     { 
      _centralLog.Log(Targets.Database, LogType.Error, $"LoadSettings error: Could not load the settings", new List<Exception>() { ex }); 
     } 
    } 
} 

Während der loadsettings Funktion Ich möchte die _settingDictionary sperren, so dass GetSetting gesperrt werden, bis die neuen Einstellungen geladen werden.

Soll ich stattdessen ein dediziertes Sperrobjekt verwenden?

Zum Beispiel:

private static readonly object m_Lock = new object(); 
… 
lock (m_Lock) 

EDIT
Ich dachte, dass Schloss (_settingDictionary) würde die _settingDictionary sperren sich jedoch merke ich jetzt, dass seine nicht der Fall ist. Was ich wollte, war zu verhindern, dass andere Threads auf _settingDictionary zugreifen, bis die neuen Einstellungen geladen wurden (LoadSettings-Methode abgeschlossen). Da nur 1 Thread das _settingDictionary aktualisiert, denke ich, dass ich dort keine Sperre brauche.

Zum Schließen der Frage - etwas ähnliches wurde schon einmal gefragt, ja, aber das Szenario ist nicht das gleiche. Ich habe aus deinen Antworten gelernt, und es wird schwer sein, einen Gewinner unter euch zu finden.

Answer 1

Dies ist ein ziemlich breites Thema, aber lassen Sie mich auf ein großes Problem in Ihrem Code konzentrieren: Änderungen.

Sie sperren das Feld nicht, Sie sperren die Instanz. Das heißt, wenn Sie auf _settingDictionary sperren, und dann Sie ändern Sie_settingDictionary, verhindern Sie keinen gleichzeitigen Zugriff - jeder kann auf dem neuen_settingDictionary sperren.

lock verhindert nicht den Zugriff auf das Objekt, das Sie sperren. Wenn Sie eine Synchronisierung benötigen, müssen Sie alle Zugriff auf das Objekt, einschließlich Ihrer _settingDictionary.TryGetValue synchronisieren. Dictionary ist nicht threadsicher.

Die wichtigsten Führungslinien zu dem, was Sie erfassen sollte wie folgt sind:

• Das Sperrobjekt ist in den Umkleide privat - wenn es nicht privat ist, könnte eine andere Klasse eine Sperre abhalten auf Objekt, das zu Deadlocks führen kann. Das Feld sollte readonly sein - das ist keine strenge Anforderung, aber es macht die Dinge einfacher. Der Hauptpunkt ist, dass Sie ein Objekt nicht sperren dürfen, das sich ändern könnte, während die Sperre gehalten wird. Andere, die versuchen, die Sperre gleichzeitig zu übernehmen, werden erfolgreich.
• Das Sperrobjekt ist ein Referenztyp - diese Art von versteht sich von selbst, aber Sie können z. ein int Feld, da es eingerahmt ist, wenn Sie versuchen, es zu sperren - in der Tat, das ist der gleiche wie der vorherige Punkt - jeder sperrt ihre eigene Instanz des Objekts, wodurch alle Synchronisierung.

obligatorische Haftungsausschluss: Multi-Threading ist hart. Ernsthaft hart. Stellen Sie sicher, dass Sie verstehen, was passiert und was möglicherweise passieren kann. Jeder Multi-Thread-Code, den Sie schreiben, muss in erster Linie so geschrieben sein, dass korrekt ist. http://www.albahari.com/threading/ ist ein guter Starter für alle Dinge multi-threaded in C# /. NET.

Answer 2

Das Problem, über das Sie sprechen, passiert, wenn Sie ein Objekt sperren, auf das externe Benutzer Ihrer Klasse Zugriff haben - am häufigsten das Objekt selbst, d. H. lock (this).

Wenn Ihr Code auf this statt _settingDictionary Sperren wurden, jemand anderes Ihren Code Deadlock könnte wie folgt aussehen:

TingbogSettingService svc = ... 
lock (svc) { 
    Task.Run(() => { 
     svc.LoadSettings(); 
    }); 
} 

Wenn Sie ein eigenes Objekt, wie _settingDictionary in Ihrem Fall auf sperren, da schädliche Wirkung beschrieben oben wird vermieden, weil niemand außerhalb Ihres Codes das gleiche Objekt sperren kann.

Hinweis: die Sperre in Ihrem Code verwendet nicht die Make-Thread-sicher, da GetSetting Methode nicht auf _settingDictionary nicht sperren, wenn von ihm zu lesen. Darüber hinaus macht die Tatsache, dass Sie innerhalb der lock_settingDictionary erneut Assessment macht irrelevant, weil nach der Neuzuweisung ein anderer Thread geschützter Abschnitt in der Sperre eingeben kann.

Answer 3

Es ist besser, ein dediziertes Objekt zu verwenden, das nicht durch den Codeblock geändert oder für andere Zwecke in anderen Methoden verwendet wird. Auf diese Weise hat das Objekt eine einzige Verantwortung, so dass Sie es nicht als Synchronisationsobjekt verwenden, da es möglicherweise irgendwann auf Null gesetzt oder durch eine andere Methode neu initialisiert wird.

lock (_settingDictionary) sperrt das zwischen() definierte Wörterbuch nicht, es sperrt den nächsten Codeblock, indem es _settingDictionary als Synchronisationsobjekt verwendet (Um zu wissen, ob der Block von einem anderen Thread links eingegeben wurde, indem man einige Flags darauf setzt Objekt).

Answer 4

Es gibt keine "richtige" oder "falsche" Antwort, aber es gibt einige Richtlinien und einige Dinge, die Sie beachten sollten.

Erstens gibt es viele, die glauben, dass Microsoft niemals willkürliche Objekte sperren darf. Stattdessen sollten sie die Sperrfunktionalität in eine bestimmte Klasse eingekapselt haben und möglichen Overhead in jedem anderen Objekt vermieden haben.

Das größte Problem beim Sperren von beliebigen Objekten besteht darin, dass Sie, wenn Sie ein Objekt sperren, das Sie einem Drittanbieter-Code öffentlich zugänglich machen, keinen Einfluss darauf haben, wer sonst das gleiche Objekt sperrt. Du könntest deinen Code auf den Buchstaben schreiben, punktierst jedes I und es würde immer noch enden, weil irgendein anderer Code von Drittanbietern das gleiche Objekt außerhalb deiner Kontrolle sperrt.

Also dieser Punkt allein ist Richtlinie genug zu sagen "Do not jemals Sperre für Objekte, die Sie öffentlich zugänglich machen".

Aber was, wenn das Objekt, auf das Sie den Zugriff synchronisieren möchten, privat ist? Nun, dann wird es mehr verschwommen. Vermutlich haben Sie die volle Kontrolle über den Code, den Sie selbst schreiben, und wenn Sie dann beispielsweise das Wörterbuch sperren, wird es gut funktionieren.

---

Dennoch mein Rat wäre, immer ein separates Objekt einrichten sperren auf, in diese Gewohnheit, und dann werden Sie nicht so leicht Fehler machen, wenn Sie später ein zuvor privates Objekt zu belichten entscheiden in die Öffentlichkeit und vergessen, die Sperrensemantik davon zu trennen.

Das Objekt einfachste Verriegelung ist nur, dass ein object:

private readonly object _SomethingSomethingLock = new object(); 

---

auch wissen, obwohl ich Sie schon denken, dass nicht „lock das Objekt“ auf ein Objekt zu verriegeln. Jeder andere Code, der sich nicht um Sperren kümmert, kann weiterhin auf das Objekt zugreifen.

---

Hier ist auch etwas, das ich gerade über Ihren Code bemerkt habe.

Wenn Sie dies tun:

lock (x) 

Sie nicht x sperren auf, sperren Sie auf dem Objekt dass x zu zum Zeitpunkt des Schlosses bezieht.

Dies ist wichtig, wenn an diesem Code suchen:

lock (_settingDictionary) 
{ 
    _settingDictionary = settings.ToDictionary(x => x.SettingName); 
}     

Hier haben Sie zwei Objekte im Spiel:

1. Das Wörterbuch, dass settingDictionary auf dem lock (_settingDictionary) zum Zeitpunkt bezieht sich
2. Das neue Wörterbuch .ToDictionary(...) gibt

zurück

Sie haben eine Sperre für das erste Objekt, aber nicht für die zweite. Dies ist ein weiteres Szenario, in dem ein dediziertes Sperrobjekt nicht nur Sinn ergibt, sondern auch korrekt ist, da der obige Code meines Erachtens fehlerhaft ist.

Answer 5

Es gibt verschiedene, was Sie konnte lock:

• ein gewidmet nicht statisches Objekt: private readonly object m_Lock = new object();
• ein gewidmet statisches Objekt (Ihr Beispiel): private static readonly object m_Lock = new object();
• das Objekt selbst : lock (_settingDictionary)
• this, typeof(MyClass) ...

Die ersten beiden sind in Ordnung, aber eigentlich anders. Das Sperren eines statischen Objekts bedeutet, dass das Schloss für alle Instanzen Ihrer Klassen freigegeben ist. Das Sperren eines nicht statischen Objekts bedeutet, dass das Schloss für jede Instanz Ihrer Klasse unterschiedlich ist.

Die dritte Option ist OK, es ist die gleiche wie die erste. Der einzige Unterschied besteht darin, dass das Objekt nicht schreibgeschützt ist (die Verwendung eines schreibgeschützten Feldes ist etwas besser, da Sie sicherstellen, dass es sich niemals ändert).

Die letzte Option ist eine schlechte Wahl aus verschiedenen Gründen, siehe Why is lock(this) {...} bad?

etwa Also seien Sie vorsichtig, was Sie sperren, Ihr Beispiel ein statisches Objekt verwendet, während der ursprüngliche Code ein nicht-statisches Objekt verwendet. Das sind wirklich unterschiedliche Anwendungsfälle.