Laden...

[gelöst] Threadsicherer Zugriff auf Dictionary funktioniert trotz lock nicht

Erstellt von DFDotNet vor 12 Jahren Letzter Beitrag vor 12 Jahren 3.249 Views
D
DFDotNet Themenstarter:in
201 Beiträge seit 2007
vor 12 Jahren
[gelöst] Threadsicherer Zugriff auf Dictionary funktioniert trotz lock nicht

Halllo,

ich habe gerade ein merkwürdiges Problem beim Zugriff auf ein Dictionary, das ich als Cache nutzen möchte um eine bestimmte Operation nicht unnötigerweise mehrmals ausführen zu müssen, wennn das Ergebnis schon bekannt ist.
Der Zugirff auf den Inhalt des dictionaries kann aus mehreren Threads erflgen, darum habe ich das ganze in ein lock() gepackt um Inkonsistenzen zu vermeiden. Sowohl die Abfrage, ob der Schlüssel schon vorhanden ist, als auch das Einfügen des Schlüssel-Wert-Paares stehen innerhalb des lock-Statements:


   private Dictionary<uhsort, ISettings> _cache = new Dictionary<ushort, ISettings>();
   private readonly object SEMAPHORE = new object();

    public ISettings GetSettings(ushort id)
    {
      lock (SEMAPHORE)
      {
        if (_cache.ContainsKey(id))
        {
          return _cache[id]; //return cached result
        }
        else //not yet in cache
        {
          ISettings result = fetchSettings(id); //add new entry to cache

          _cache.Add(id, result); //ArgumentException occurs here! (key already existing)
          return result;
        }
      }
    }

Trotzdem tritt beim Einfügen neuer Einträge manchmal eine ArgumentException auf:> Fehlermeldung:

An item with the same key has already been added.

Es wird an keiner anderen Stelle auf das Dictionary zugegriffen, auch das lock-object wird nirgends sonst verwendet.
Kann mir jemand erklären, wie das sein kann? Habe ich die Arbeitsweise von lock() doch nich nicht richtig verstanden?

Danke!

T
2.219 Beiträge seit 2008
vor 12 Jahren

Liegt wohl daran, dass deine Verarbeitung nicht Threadsicher ist.

Nutze für solche Sachen static.
Setze die Methoden und die Member der Klasse auf static, dann ist deine Methode Threadsicher.

Nachtrag:
Meine Vermutung ist, dass jeder Thread eine eigene Instanz des Lock Objekts bekommt.
Deshalb kann auch Problemlos mehrfach auf das Dictionary zugegriffen werden.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

C
2.121 Beiträge seit 2010
vor 12 Jahren

Setze die Methoden und die Member der Klasse auf static, dann ist deine Methode Threadsicher.

Nee?! Die kann dann trotzdem mehrfach parallel aufgerufen werden!

Das einzige was static sein muss ist das lock Objekt.

49.485 Beiträge seit 2005
vor 12 Jahren

Hallo chilic,

das die Ressource, auf die der Zugriff erfolgt, also das Dictionary, nicht static ist, muss auch das lock-Objekt nicht static sein. Es würde im Gegenteil zu unnötigen gegenseitigen Blockierungen führen, wenn es das wäre.

Hallo DFDotNet,

irgendeine der von dir getroffenen Annahmen wird falsch sein. Sonst würde keine Exception kommen.

herbivore

T
2.219 Beiträge seit 2008
vor 12 Jahren

Was mir auf die schnelle noch einfallen könnte ist, dass die id doppelt vorhanden ist.
Wie wird diese ermittelt/generiert?
Dort wird es noch einen Fehler geben bzw. gibt es beim auslesen der Daten ggf. doppelte ID's.

Nachtrag:
Könntest du ggf. noch zeigen wie der Aufruf deiner Methode durchgeführt wird bzw. wie die gesamte Verarbeitung an der Stelle ist?
Scheinbar läuft dort irgendwas nicht rund.

Nachtrag2:
Anbei enthält dein Code oben einen Tippfehler beim Dictionary.
Ist dies auch der richtige Code oder ist dies nur schnell nachgetippt?

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

49.485 Beiträge seit 2005
vor 12 Jahren

Hallo T-Virus,

mehrfach auftauchende IDs werden durch die ContainsKey-Abfrage abgefangen ... oder sollten es zumindest.

herbivore

T
2.219 Beiträge seit 2008
vor 12 Jahren

Ist mir auch klar geworden nachdem ich den Code nochmal angeschaut hatte.
Mir gehen langsam die Ideen aus wie der Fehler auftretten kann.
Meine Vermutung ist immer noch, dass die Verwendung seines Codes zu einer nicht Threadsicheren Verwendung führt oder das der gezeigte Code nicht dem entspricht was er eigentlich nutzt.

Da ein Tippfehler im Code bei der Deklaration vom Dictionary drin ist, lässt sich der Code auch nicht kompilieren.
Deshalb wäre der richtige Code samt dem Beispiel wo er aufgerufen wird hilfreicher.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

16.806 Beiträge seit 2008
vor 12 Jahren

Ich würde hier, sofern es die .NET Version zulässt, auf das ConcurrentDictionary zurückgreifen, das Thread-sicher ist.
Vermute auch, dass etwas das Problem ist, das wir hier gar nicht im Code zu sehen bekommen.

C
2.121 Beiträge seit 2010
vor 12 Jahren

Herbivore hat recht, das Lockobjekt muss nicht static sein. Eigentlich könnte man hier auch gleich das Dictionary selbst verwenden.
Ich finds seltsam dass innerhalb der paar wenigen Instruktionen (bis auf die Methode fetch...) so oft "irgendjemand" die ID einfügen kann, dass es hier öfter zu Problemen kommt.
Mach die Abfrage mit dem Contains nochmal nach dem fetch und zähl wie oft das vorkommt.
Das einzige was ich mir vorstellen könnte ist, dass fetchsettings das Dictionary verändert. Eigentlich sollte nämlich ja kein anderer Thread da ran kommen.

Falls nicht wirds schwieriger.
Dann könntest du mal die ganzen Keys vor dem ersten Test und beim Add irgendwohin loggen (reicht ja in eine Variable) und die dann im Debugger vergleichen. Um zu sehen wie sehr sich das Dictionary unterscheidet. Evtl. gibt das Aufschluss?

Oder leite ein eigenes Dictionary ab und überschreibe die Add-Methode, so dass die mitloggt wann sie welchen key schreibt. Vielleicht hilft ja das noch weiter.

5.657 Beiträge seit 2006
vor 12 Jahren

IVermute auch, dass etwas das Problem ist, das wir hier gar nicht im Code zu sehen bekommen.

Vermute ich auch. Deswegen würde ich vorschlagen, mal an der Stelle einen Breakpoint zu setzen und dann zu schauen, woher der doppelte Eintrag kommt.


 public ISettings GetSettings(ushort id)
    {
      lock (SEMAPHORE)
      {
        if (_cache.ContainsKey(id))
        {
          return _cache[id]; //return cached result
        }
        else //not yet in cache
        {
          ISettings result = fetchSettings(id); //add new entry to cache
          
            if (_cache.ContainsKey(id))
                Console.Write("dsgdg"); // Hier den Brakpoint setzen!

          _cache.Add(id, result); //ArgumentException occurs here! (key already existing)
          return result;
        }
      }
    }

Wenn die Verarbeitung dort anhält, kannst du dich im StackTrace nach oben hangeln, und siehst die Methodenaufrufe, den den Wert in den Cache schreiben. Sicherlich passiert das irgendwo innerhalb der fetchSettings-Methode.

Christian

Weeks of programming can save you hours of planning

D
DFDotNet Themenstarter:in
201 Beiträge seit 2007
vor 12 Jahren

Hallo,

erstmal Vielen Dank für die vielen Antworten.
Zuerst dachte ich, ich hätte eine ganz blöden Fehler gemacht, aber die weiteren Beiträge haben dann ja gezeigt, dass es doch nicht so trivial zu sein scheint.

@T-Virus:
Ja, der Code ist nur schnell nachgetippt und auf die relevanen Stellen beschränkt. Alles andere (aufrufende Klassen etc) wäre zu unübersichtlich und spielt auch keine Rolle.
Die relevanten Teile sind wirklich alle enthalten. Einen zweiten Aufruf an irgendeiner anderen Stelle, an der etwas ins dictionary gepackt wird o.ä., gibt es nicht. Das habe ich mit der Symbolsuche mehrmals überprüft. Das Dictionary bearbeitet niemend sonst.

Ich finds seltsam dass innerhalb der paar wenigen Instruktionen (bis auf die Methode fetch...) so oft "irgendjemand" die ID einfügen kann, dass es hier öfter zu Problemen kommt.
Mach die Abfrage mit dem Contains nochmal nach dem fetch und zähl wie oft das vorkommt.

Es ist noch seltsamer.
Es ist nämlich so, dass gar nicht viel passiert, bis der Fehler auftritt. Die Methode wird einmal aufgerufen, dann wird richtigerweise der erste Eintrag erstellt.
Gleich beim 2. Aufruf tritt dann der Fehler auf. Danach aber nie wieder.

Das einzige, was mir noch einfällt, was evtl interessant sein könnte, ist dass die methode fetchsettings() beim 1. Aufruf u.U. länger braucht, als bei den späteren Aufrufen, da sie ihrerseits auf eine andere Resource zugreift, die beim 1. Aufruf initialisiert werden muss.

Ich werde gleich mal versuchen das Problem mit einem kleinen eigenständigen Tool nachzustellen, ohne das ganze Framework was hier noch drumherum ist.

16.806 Beiträge seit 2008
vor 12 Jahren

Es ist in diesem Fall schon recht trivial.
Aber ich versteh nicht, wieso Du nicht einfach die Dinge machst, die man Dir empfiehlt; zB MrSparkle.
Lieber erfindest das Rad neu..?

Zudem die Aussage von herbivore

irgendeine der von dir getroffenen Annahmen wird falsch sein. Sonst würde keine Exception kommen.

Ich bezweifel, dass es ein hochkomplizierter und nicht nachvollziehbarer Fehler ist.

C
2.121 Beiträge seit 2010
vor 12 Jahren

Die Methode wird einmal aufgerufen, dann wird richtigerweise der erste Eintrag erstellt.
Gleich beim 2. Aufruf tritt dann der Fehler auf. Danach aber nie wieder.

Dann wärs doch ne Idee das ganze mal zu debuggen und zu schauen, was im Dictionary drin steht wenn CntainsKey aufgerufen wird (das dann false zurückliefert?) und was drin steht wenn der Eintrag nochmal geschrieben werden soll.

D
DFDotNet Themenstarter:in
201 Beiträge seit 2007
vor 12 Jahren

Es ist in diesem Fall schon recht trivial.
Aber ich versteh nicht, wieso Du nicht einfach die Dinge machst, die man Dir empfiehlt; zB MrSparkle.
Lieber erfindest das Rad neu..?

Sorry, das hatte ich nicht erwähnt. Das hatte ich vorher schon gemacht.

Der StackTrace zeigt mir, dass die Methode von 2 auf dem GUI sichtbaren Elementen aufgerufen wird. Es gibt keine Stelle (außer der geposteten) an der das Dictionary verändert wird.

Zudem die Aussage von herbivore

irgendeine der von dir getroffenen Annahmen wird falsch sein. Sonst würde keine Exception kommen.

Ich bezweifel, dass es ein hochkomplizierter und nicht nachvollziehbarer Fehler ist.

Das sehe ich eigentlich genauso. Darum habe ich meine Aussagen ja auch mehrmals überprüft. Dummerweise bin ich mir mittlerweile ziemlich sicher, dass sie tatsächlich stimmen. Das verwirrt mich umso mehr.

Dann wärs doch ne Idee das ganze mal zu debuggen und zu schauen, was im Dictionary drin steht wenn CntainsKey aufgerufen wird (das dann false zurückliefert?) und was drin steht wenn der Eintrag nochmal geschrieben werden soll.

Das habe ich auch getan. Wie erwartet, ist der Inhalt zunächst leer, in dem Moment, wo der Wert geschrieben werden soll, aber nicht.

Ich werde wohl mal deinen Vorschlag angehen ein eigenes Dictionary abzuleiten und darin zu debuggen. Mal sehen, was das ergibt

D
DFDotNet Themenstarter:in
201 Beiträge seit 2007
vor 12 Jahren

So, das habe ich jetzt auch mal gemacht. Beim überprüfen des StackTrace ist aber auch nichts weiter herausgekommen, als vorher. Add wird immer nur aus der Mehtode heraus aufgerufen, die ich schon gepostet hatte. Im StackTrace den ich in der Add()-Methode bekomme ist fetchSettings() NICHT enthalten.
Aber was mich stutzig gemacht hat:
Wenn ich in der Methode Add (oder auch außerhalb, also in die Zeile, wo Add aufgerufen wird) einen Brakpoint setze, sehe ich, dass die Zeile fetchSettings(id) darüber grau hinterlegt ist, also offenbar gerade ausgeführt wird. Allerdings ist sie wie gesagt nicht im aktuellen StackTrace enthalten. Also wird sie wohl in einem anderen Thread gerade ausgeführt.

Um das zu überprüfen habe ich mal testweise folgendes Konstrukt eingefügt:



private bool _executingMethod = false;

public ISettings GetSettings(ushort id)
{
  lock (_cache)
  {
     try
     {
       if (_executingMethod)
       {
         throw new Exception("This is not possible !!!"); //diese Stelle wird erreicht!
       }
       _executingMethod = true;
    
       //hier steht wieder der normale Methoden-Inhalt
       //(zur Übersichtlichkeit weggelassen)
       ...

     }
     finally
     {
       _executingMethod = false;
     }
  }
}

Das mag etwas merkwürdig aussehen, aber es zeigt anhand der Stelle mit der Exception dass die Methode 2x parallel ausgeführt wird.
Das kann nach meinem Verständnis nicht sein, wenn mittels lock() synchronisiert wird, oder?

49.485 Beiträge seit 2005
vor 12 Jahren

Hallo DFDotNet,

Das kann nach meinem Verständnis nicht sein, wenn mittels lock() synchronisiert wird, oder?

das kann dann sein, wenn das Objekt, das von _cache referenziert wird, beim zweiten Aufruf ein anderes ist. Oder wenn executingMethod außerhalb der Methode vor dem ersten Aufruf auf true gesetzt wird.

Bau ein Projekt, dass nur den absolut nötigen Code enthält. Ersetze dass Fetch durch eine leere (Warte-)Schleife.

herbivore

D
DFDotNet Themenstarter:in
201 Beiträge seit 2007
vor 12 Jahren

Hallo herbivore,

das kann dann sein, wenn das Objekt, das von _cache referenziert wird, beim zweiten Aufruf ein anderes ist.

Ist nicht der Fall. Das objekt wird nur einmal erzeugt und zugewiesen:


private Dictionary<ushort, ISettings> _cache = new Dictionary<ushort, ISettings>();

Oder wenn executingMethod außerhalb der Methode vor dem ersten Aufruf auf true gesetzt wird.

Ist nicht der Fall. Die variable habe ich nur zum testen eingeführt und sie wird nirgends sonst außer an den gezeigten Stellen verwendet.

Bau ein Projekt, dass nur den absolut nötigen Code enthält. Ersetze dass Fetch durch eine leere (Warte-)Schleife.

Das werde ich jetzt machen.
Sehr merkwürdig alles...

5.657 Beiträge seit 2006
vor 12 Jahren

Wenn ich in der Methode Add (oder auch außerhalb, also in die Zeile, wo Add aufgerufen wird) einen Brakpoint setze, sehe ich, dass die Zeile fetchSettings(id) darüber grau hinterlegt ist, also offenbar gerade ausgeführt wird. Allerdings ist sie wie gesagt nicht im aktuellen StackTrace enthalten. Also wird sie wohl in einem anderen Thread gerade ausgeführt.

Dann bist du doch auf dem richtigen Weg, bzw. hast das Problem ja fast gefunden. Du mußt jetzt nur den restlichen StackTrace überprüfen bzw. einfach mal per Einzelschrittdebugging verfolgen, wann die fetchSettings-Methode die Add-Methode aufruft.
Christian

Weeks of programming can save you hours of planning

5.742 Beiträge seit 2007
vor 12 Jahren

sehe ich, dass die Zeile fetchSettings(id) darüber grau hinterlegt ist, also offenbar gerade ausgeführt wird. Allerdings ist sie wie gesagt nicht im aktuellen StackTrace enthalten.

Dann schaue einmal in die anderen Threads, wenn das auftritt (Doppelklick auf den entsprechenden Thread im Fenster "Threads").

D
DFDotNet Themenstarter:in
201 Beiträge seit 2007
vor 12 Jahren
[gelöst] Threadsicherer Zugriff auf Dictionary funktioniert trotz lock nicht

Hallo,

sorry, dass ich jetzt erst daruf antworte, ich kam erst heute morgen wieder dazu nach der Sache zu schauen. Ich habe das Problem aber schließlich doch noch gefunden und beseitigen können.
Ein anderer Thread war es nicht. Das hätte mich auch gewundert, denn das sollte ja durch das lock-Statement abgefangen sein.

Ich konnte es jetzt soweit zurückverfolgen, dass ich an den Anfang und das Ende von fetchSettings() je einen Breakpoint gesetzt habe.
Beim 1. Aufruf der Methode wurde der Breakpoint am Anfang schon das 2. mal erreicht, bevor der Am Ende das erste mal erreicht wurde.
Da musste also irgendwo der Haken liegen. Ich habe dann herausfinden können, dass durch "irgend"ein event in der Rahmenanwendung das durch das 'fetchen' der settings ausgelöst wird, eine andere Komponente von mir dazu gebracht wird, die Methode GetSettings() nocheinmal aufzurufen. Der 2. Aufruf kam also aus dem selben Thread und geschah (wenn auch indirekt) innerhalb des 1. Aufrufes.
Allerdings nur beim 1. mal, so dass keine endlose Rekrsion entstand. Durch das weiterleiten über mehrere events war der StackTrace aber so unübersichtlich, dass man gar nicht mehr sehen konnte, dass die Aufrufe ineinander geschachtelt waren, man sah nur noch graue DynamicInvoke- und Invoke-statements im StackTrace-Fenster. Jedenfalls konnte ich dadurch, dass ich meinen anderen EventHandler erst zu einem späteren Zeitpunkt registriere, das Problem beheben.

Vielen Dank für eure Tipps, die den Denkanstoß für die Suche in die richtige Richtung gegeben haben!