List<> und Threadsafety



  • Ape schrieb:

    ...

    👍 Besser kann man sich keinen Deadlock bauen.



  • @GPC:

    Das ist ja nur Beispiel-Code 😉
    Man sollte natürlich einen Try-Catch-Block um den Zugriff setzen. Aber eigentlich macht es keinen Sinn das Programm weiterlaufen zu lassen, wenn hierbei ein Fehler auftreten solle. 😛

    Von "lock" habe ich bis jetzt nur gehört, dass es das Programm ziemlich ausbremst. Wenn du es verwenden willst, solltest du dich vielleicht über die Vor- und Nachteile erkundigen. 🙂



  • Ape schrieb:

    Von "lock" habe ich bis jetzt nur gehört, dass es das Programm ziemlich ausbremst. Wenn du es verwenden willst, solltest du dich vielleicht über die Vor- und Nachteile erkundigen. 🙂

    Der Nachteil an deinem Code ist halt, dass er nicht funktioniert.



  • Ape schrieb:

    Aber eigentlich macht es keinen Sinn das Programm weiterlaufen zu lassen, wenn hierbei ein Fehler auftreten solle. 😛

    Die anderen Threads wären sicher gerne weitergelaufen, wenn die Exception in einem einzelnen Thread sie nicht blockieren würde 😉

    Von "lock" habe ich bis jetzt nur gehört, dass es das Programm ziemlich ausbremst. Wenn du es verwenden willst, solltest du dich vielleicht über die Vor- und Nachteile erkundigen. 🙂

    Oh, ich will dir nicht zu Nahe treten, aber machen wir es doch andersherum: Du erkundigst dich erstmal über lock und dann diskutieren wir die Vor- und Nachteile deiner "Lösung" 😉
    Zum Anfang wäre das hier geeigneter Lesestoff: http://blog.coverity.com/2014/02/12/how-does-locking-work/



  • inflames2k schrieb:

    👍 Besser kann man sich keinen Deadlock bauen.

    Ich verwende dieses Verfahren bei einer TCP/IP-Kommunikation und hatte bisher noch keine Probleme damit. Der Fall, dass ein Thread warten muss, tritt ja eigentlich nur selten auf, da Threads nicht all zu oft auf ein Objekt zur exakt selben Zeit zugreifen. Kommt natürlich auf den Anwendungsfall an. Und falls er dann mal doch in die Schleife gerät, sollte er ja im Regelfall nach dem ersten Schleifen-Durchlauf weitermachen. Wenn 1ms zu viel sein sollte, kann man das Thread.Sleep auch ganz weglassen.



  • Ape schrieb:

    Ich verwende dieses Verfahren bei einer TCP/IP-Kommunikation und hatte bisher noch keine Probleme damit.

    Natürlich, es geht so lange gut, bis es eben knallt.

    Sobald dein Code ein wenig unter Last gerät, schmiert er ab. Immer mal wieder, unvorhersagbar.

    Was meinst du, warum es fertige Klassen für Synchronisierung gibt? Weil man sie nicht braucht, sondern sich mit einem bool und einer Schleife das gleiche eben mal hinbasteln kann?



  • Sorry Ape aber Dein Code ist wirklich übel. Am besten direkt wegwerfen und erstmal in Threachsychronisation reinlesen.

    Ape schrieb:

    Man sollte natürlich einen Try-Catch-Block um den Zugriff setzen.

    Und Exception-Handling hast Du auch nicht verstanden.



  • Wie gesagt habe ich mich selbst mit "lock" noch nicht viel befasst. Ich will ja damit nicht sagen, dass es schlecht ist, sondern dass man es sich mal genauer anschauen sollte, bevor man es einfach verwendet.

    Ihr erwähnt dauernd, dass es mit dem Flag nicht auf Dauer funktioniert. Kann bitte einer von euch auch erklären warum? Habe zwar schon einen Stresstest bei der TCP/IP-Kommunikation getestet und nichts festgestellt, aber ich würde gerne mal wissen, was da schieflaufen kann. 😕

    Ich will ja nicht behaupten, dass mein Verfahren das einfachste und beste ist und alle anderen kacke und unnötig. 😃 Bin ja selbst noch am Lernen und wär über andere überzeugende Vorschläge dankbar, damit ich meinen Code anpassen kann. Aber einfach nur sagen, dass es nicht klappt, ist nicht sehr hilfreich. 😞



  • Ape schrieb:

    Das ist ja nur Beispiel-Code 😉

    Ich halte das für ein Anti-Beispiel Code. So sollte man es auf keinen Fall machen. Vor allem weil der Code definitiv nicht thread-safe ist, wie andere schon erwähnten.

    Ape schrieb:

    Ich verwende dieses Verfahren bei einer TCP/IP-Kommunikation und hatte bisher noch keine Probleme damit.

    So ziemlich alles funktioniert _bis_ zu dem Moment wo es dann schiefgeht. Leider verwechseln die meisten möchtegern-Progger ein "ist bisher noch nicht schiefgegangen" mit "funktioniert korrekt".

    Das ist wie der Fallschirmspringer der ohne Schirm rausspringt. Die ersten 90 Sekunden funktioniert das auch problemlos. Trotzdem bezweifele ich das er es richtig gemacht hat...



  • Ape schrieb:

    Ihr erwähnt dauernd, dass es mit dem Flag nicht auf Dauer funktioniert. Kann bitte einer von euch auch erklären warum?

    Zwei Threads. Der eine ruft Add auf, der andere Get. Beide laufen an deiner Synchronisierungsschleife vorbei (wirdVerwendet ist ja false). Beide Threads greifen gleichzeitig auf die Liste zu -> Peng.

    Selbst wenn dein Programm auf einem einzigen Kern ausgeführt wird, und die Threads somit nicht gleichzeitig ablaufen können, kann dir das passieren:

    Thread 1 ruft Add auf, läuft an der Schleife vorbei.
    Wechsel nach Thread 2, bevor Thread 1 wirdVerwendet setzen kann.
    Thread 2 ruft Get auf, läuft auch an der Schleife vorbei, setzt wirdVerwendet, und durchsucht die Liste. Mittendrin wieder Wechsel zu Thread 1.
    Thread 1 setzt auch wirdVerwendet, und ändert die Liste.
    Wechsel zu Thread 2, der mit dem Zugriff auf eine Liste fortfährt, die zwischenzeitlich geändert wurde -> Peng.



  • Okay, also ein lock wäre dann genauso safe wie die Verwendung von zum Beispiel
    ConcurrentQueue? Irgendwie komm ich ohne den Indexzugriff bei einer Liste nicht
    zurecht.

    Im Konkreten Fall habe ich ein Objekt List<List<Point>>, das übergeben werden
    soll. ConcurrentQueue<List<Point>> wäre dann aber auch unsinnig, wenn die
    innere Liste gleichzeitig verändert und darauf zugegriffen wird, oder?



  • Ape schrieb:

    @GPC:

    Das ist ja nur Beispiel-Code 😉
    Man sollte natürlich einen Try-Catch-Block um den Zugriff setzen. Aber eigentlich macht es keinen Sinn das Programm weiterlaufen zu lassen, wenn hierbei ein Fehler auftreten solle. 😛

    Von "lock" habe ich bis jetzt nur gehört, dass es das Programm ziemlich ausbremst. Wenn du es verwenden willst, solltest du dich vielleicht über die Vor- und Nachteile erkundigen. 🙂

    NEIN DAS IST KEIN BEISPIELCODE, ES IST FALSCH, UND ES IST GEFÄHRLICH.

    Wenn man keine Ahnung von etwas hat, dann sollte man es bleiben lassen. Und wenn man schon unbedingt solchen Unfug bauen muss, dann sollte man ihn gefälligst nicht in Foren posten.



  • Ape schrieb:

    Ihr erwähnt dauernd, dass es mit dem Flag nicht auf Dauer funktioniert. Kann bitte einer von euch auch erklären warum?

    Der erste "Bug" ist, dass du nicht volatile verwendest. Dadurch muss der Compiler an der Stelle wo du die Variable zuweist gar keine echte Zuweisung machen. Unter bestimmten Bedingungen, z.B. dass in dem Teil zwischen "wirdVerwendet = true" und "wirdVerwendet = false" keine Exceptions geworfen werden können, kann der Compiler beide Zuweisungen einfach weglassen. Der Compiler "sieht" dann beim Optimieren:
    * Wenn wir aus der Schleife rauskommen muss die Variable false sein, sonst wären wir nicht rausgekommen.
    * Dann schreiben wir true rein.
    * Dann machen wir was.
    * Dann schreiben wir wieder false rein.
    * Dazwischen liest keiner die Variable (da die Variable nicht volatile ist, müssen andere Threads nicht berücksichtigt werden).
    => Wir können die zwei Zuweisungen gleich ganz weglassen.

    Selbst mit volatile hast du aber das Problem, dass zwei Threads gleichzeitig in einer Funktion warten könnten, und dann beide gleichzeitig "sehen" dass "wirdVerwendet" false ist, und beide gleichzeitig weiterlaufen.
    Und dann greifen erst wieder zwei (oder evtl. sogar mehr) Threads gleichzeitig auf die Collection zu.
    Zusätzlich setzt dann noch der Thread der als erstes bei "seinem" "wirdVerwendet = false" ankommt die Variable auf false, und weitere Threads die warten laufen ebenfalls los.

    Um das zu lösen brauchst du dann sog. atomare Operationen, und zwar im speziellen eine "CAS" Operation (Compare And Swap). Im .NET Framework sind diese atomaren Operationen in der Interlocked Klasse zu finden. Die "CAS" Funktion heisst dort Interlocked.CompareExchange.
    http://msdn.microsoft.com/en-us/library/system.threading.interlocked.compareexchange.aspx

    Und wenn du damit dann alle Bugs beseitigt hast, dann hast du nen Code der mit an Sicherheit grenzender Wahrscheinlichkeit langsamer ist als lock() .
    Weil lock() im Endeffekt nämlich auch nix anderes macht. Nur dass es von Profis entwickelt wurde und sehr gut optimiert ist.

    => Lass den Unsinn, und verwende die fertigen Funktionen die genau dafür gemacht wurde.



  • huddi schrieb:

    Okay, also ein lock wäre dann genauso safe wie die Verwendung von zum Beispiel
    ConcurrentQueue? Irgendwie komm ich ohne den Indexzugriff bei einer Liste nicht
    zurecht.

    Wenn dem wirklich so ist, verweise ich nocheinmal auf meinen ersten Beitrag. - Dort habe ich meine ThreadSafeList<T> vorgeschlagen, die sich genauso verwenden lässt die die List<T> des Frameworks.



  • inflames2k schrieb:

    Dort habe ich meine ThreadSafeList<T> vorgeschlagen, die sich genauso verwenden lässt die die List<T> des Frameworks.

    Deine ThreadSafeList ist bloss leider überhaupt nicht thread-safe.

    Nur mal eines von vielen Beispielen:

    public bool Remove(T item)
             {
                 int index = this.IndexOf(item);
                 if (index == -1)
                     return false;
    
                 this.RemoveAt(index);
                 return true;
             }
    

    Echt jetzt?

    Überleg dir mal was passiert wenn die Liste {1, 2, 3} enthält, und dann zwei Threads gleichzeitig Remove(2) machen.

    Oder ne Liste die ein einziges Element "1" enthält. Bei zwei zeitlich getrennten Remove-Aufrufen liefert der erste true zurück und der zweite false . Bei zwei gleichzeitigen Remove-Aufrufen wird einer true zurückliefern, der zweite kann aber ne ArgumentOutOfRangeException Exception werfen.

    Also auch an dich: bitte lies dich etwas genauer in das Thema ein, und poste erst dann gute Ratschläge, wenn du dir wirklich sicher bist es verstanden zu haben.

    Und BTW: http://stackoverflow.com/questions/251391/why-is-lockthis-bad



  • Du hast dann sicher auch in die Methoden reingeschaut die in Remove aufgerufen werden oder?!

    Das mit dem lock(this) ist natürlich richtig. - Da aber von außen kein Grund besteht die Liste zu locken, find ich das jetzt nicht so wild. Außerdem steht es ja jedem frei, statt lock(this) ein Lockobjekt einzubauen.

    // EDIT: Argh... ich seh grad das Problem. Hast natürlich recht. -> Beide Threads können den Index noch abfragen. Aber davon mal abgesehen ist das wenn ich den Code richtig überflogen habe die einzige Stelle.

    // EDIT: Bezüglich lock(this) finde ich auch die Argumentationen von herbivore im Thread [url=http://www.mycsharp.de/wbb2/thread.php?threadid=47031]
    Sollte man lock this vermeiden?[/url] gut. - Ja es sind zum Teil konstruierte Fälle. Aber es bringt die Sache auf den Punkt und wir müssen hier jetzt nicht darüber diskutieren.



  • inflames2k schrieb:

    Du hast dann sicher auch in die Methoden reingeschaut die in Remove aufgerufen werden oder?!

    Ja, ist aber egal, da muss man nicht reingucken um den Fehler zu sehen.

    // EDIT: Argh... ich seh grad das Problem. Hast natürlich recht. -> Beide Threads können den Index noch abfragen. Aber davon mal abgesehen ist das wenn ich den Code richtig überflogen habe die einzige Stelle.

    Genau.

    Und nein, das ist lange nicht die einzige Stelle.

    Dein Indexer ist genau so kaputt.
    AddRange() ist kaputt wenn man die (mMn. vernünftige) Anforderung stellt, dass zwei gleichzeitig laufende AddRange Aufrufe die Elemente nicht "gemischt" einfügen.
    Und GetEnumerator ist ober-kaputt. Es ist zwar "safe", aber blockiert die Liste bis entweder der Enumerator fertig "durchgespult" oder disposed wurde.

    Und mir kommt vor dass da noch mehr Stellen waren. Gib's zu, du hast da selbst noch ein paar Bugs gefunden und gefixt.

    Und es gibt noch zwei ganz grundlegende Probleme mit dem Code (beides keine "Bugs", aber beides mMn. klare Fehlentscheidungen):

    1. Du verwendest ein nacktest Array statt List<T>. Wozu? Stehst du auf Schmerzen oder auf unnötige Arbeit?

    2. Der Versuch IList<T> thread-safe zu implementieren. IList<T> ist KEIN geeignetes Interface für eine intern synchronisierte Collection-Klasse!

    Von einer intern synchronisierten Collection erwarte ich mir Funktionen wie z.B.:
    😉 Entferne das letzte Element, und gib es mir zurück (bzw. gib mir null wenn die Liste leer war).
    😉 Suche das erste X Element und ersetze es mit Y und gib mir true/false zurück-
    😉 Suche alle X Elemente und ersetze sie mit Y, und gib mir zurück wie viele es waren.
    😉 Suche Element X, und wenn du es gefunden hast, rufe Delegate D mit X als Parameter auf, während der Lock noch gehalten wird.
    usw.
    Wenn ich keine dieser Funktionen habe, dann brauche ich auch keine intern synchronisierte Collection. Weil ich dann sowieso nochmal selbst aussenrum synchronisieren muss.

    Bzw. normale Listen-artige Collections sind als thread-safe Variante auch nicht wirklich interessant. Für Queues oder Dictionaries fallen mir dagegen einige Dinge ein wo man die thread-safe brauchen könnte.



  • hustbaer schrieb:

    Gib's zu, du hast da selbst noch ein paar Bugs gefunden und gefixt.

    Nein nicht wirklich, das einzige was ich angepasst habe war der Zugriff auf Remove(T item) und die Anpassung von lock(this) auf ein Lock-Objekt.

    hustbaer schrieb:

    Und es gibt noch zwei ganz grundlegende Probleme mit dem Code (beides keine "Bugs", aber beides mMn. klare Fehlentscheidungen):

    1. Du verwendest ein nacktest Array statt List<T>. Wozu? Stehst du auf Schmerzen oder auf unnötige Arbeit?

    2. Der Versuch IList<T> thread-safe zu implementieren. IList<T> ist KEIN geeignetes Interface für eine intern synchronisierte Collection-Klasse!

    Zu 1.: Die interne Verwendung des Arrays war eher zu Übungszwecken. Klar hätte ich intern noch eine Liste halten können, aber da hätte ich nichts gelernt. (Die Liste bestand zuerst ohne die Synchronisierung der Zugriffe).

    Zu 2.: Das ist deine Ansicht und da werde ich dir sicher nicht wiedersprechen.

    hustbaer schrieb:

    Von einer intern synchronisierten Collection erwarte ich mir Funktionen wie z.B.:
    😉 Entferne das letzte Element, und gib es mir zurück (bzw. gib mir null wenn die Liste leer war).
    😉 Suche das erste X Element und ersetze es mit Y und gib mir true/false zurück-
    😉 Suche alle X Elemente und ersetze sie mit Y, und gib mir zurück wie viele es waren.
    😉 Suche Element X, und wenn du es gefunden hast, rufe Delegate D mit X als Parameter auf, während der Lock noch gehalten wird.
    usw.
    Wenn ich keine dieser Funktionen habe, dann brauche ich auch keine intern synchronisierte Collection. Weil ich dann sowieso nochmal selbst aussenrum synchronisieren muss.

    Das geht dann aber über die Funktionalität einer reinen Liste hinaus. Insofern seis drum, war bei mir nicht die Anforderung. 🙄

    hustbaer schrieb:

    Bzw. normale Listen-artige Collections sind als thread-safe Variante auch nicht wirklich interessant. Für Queues oder Dictionaries fallen mir dagegen einige Dinge ein wo man die thread-safe brauchen könnte.

    Hier hast du natürlich recht. Aber ab und an kommt man eben doch in den Genuss Listen in mehreren Threads verwenden zu müssen.

    So und nun Thema bei Seite, besser fährt der TE natürlich wenn er eine der von Profis erstellten Collections nutzt.


Anmelden zum Antworten