Thread Safe Methode

Question

Guten Tag liebe Forumgemeinde,

ich bin momentan mit folgendem Problem beschäftigt:

public class A
{
   private B;

   public A()
   {
     B = new B();
   }

   public ValueChangedEvent(object sender, MonitoredItemNotificationEventArgs e)
   {
     B.WriteInDataBase(e.Value);
   }

}

public class B
{
   int counter = 0;
   string[] arrValues = new string[4000];

public void WriteInDatabase(string Value)
{
   arrValues[0] = Value;

   counter++;

   if(counter == 4000)
   {
     counter = 0;
     string[] tmpArray = arrValues;
     //tmpArray In die DB schreiben
   }
}

}

Das Event ValueChangedEvent der Klasse A feuert, wenn sich Daten ändern. Jeder Thread ruft jetzt die Methode

B.WriteInDataBase

der Klasse B auf.

Ich weiß selbst, dass der counter und das Array nicht ThreadSafe sind. Dennoch bin ich momentan etwas überfragt. Ich muss definitiv in die DB schreiben, wenn das Array die 4000 Einträge hat.

Würde ich den Counter und das Array jetzt lokal in der Methode direkt initialisieren, wäre das Problem ja, dass die IF Bedingung erst True ist, wenn ein Thread 4000 Array-Einträge hat.

Vielen Dank im Vorraus :)

Answer 1

Hi,
und wo ist der Thread? Welche Methoden laufen in einem anderen thread und erfordern einen sicheren threadübergreifenden Zugriff? Und warum soll counter und das Array nicht threadsicher sein?

---

--
Viele Grüsse
Peter Fleischer (MVP, Partner)
Meine Homepage mit Tipps und Tricks
Kommas richtig setzen!
Schüler sagen, Lehrer haben es gut.
Schüler, sagen Lehrer, haben es gut

Answer 2

Hi. Das Event wird im 50ms Intervall gefeuert.

Ich habe im Event mit

Thread thread = Thread.CurrentThread;
int threadID = thread.ManagedThreadId;

geschaut, welche Thread ID's sich im Event befinden. Diese waren mal 7, 19, 27 usw.

Das Problem ist, dass in

public void WriteInDatabase(string Value)

im letzten Durchlauf ein Thread den Counter auf 4000 hochzählt. Ein Thread funkt dazwischen und zählt diesen auf 4001 hoch. Somit bekomme ich bei

 arrValues[0] = Value;

eine IndexOutOfRange Exception.

Answer 3

Hi,
das hat aber nichts mit thread-übergreifender Sicherheit zu tun. Da ist in Deinem Programm eine fehlerhafte Logik. Wenn Du in einem Thread Objekte hinzufügst, die Du dann in einem anderen Thread verarbeitest, dann musst Du damit rechnen, dass bei einer längeren Verarbeitung zwischenzeitlich weitere Objekte hinzugefügt werden.

Um solche Probleme zu umgehen, musst Du beide Prozesse voneinander entkoppeln. Entweder Du kopierst zu Beginn der Verarbeitung die zu verarbeitende Menge und fügst die neuen Objekte einem leeren Container zu, oder Du nutzt einen ausreichend großen Container und prüfst in jeden Verarbeitungsschritt, ob noch etwas zu verarbeiten ist und verarbeitest ggf. die zwischenzeitlich hinzugefügten Objekte.

Da Du mit einem Array arbeitest, was bei voller Füllung keine weiteren Objekte aufnehmen kann, kommt in diesem Fall nur der erste Ansatz zum Einsatz, d.h. zu Verarbeitungsbeginn wird das alte Array entkoppelt, indem die neuen Objekte in einem neuen Array gesammelt werden. Ich finde diese Arbeitsweise aber nicht optimal, da es bei Verzögerungen der Verarbeitung passieren kann, dass das neue Array bereits voll ist, wenn das alte Array noch nicht vollständig verarbeitet wurde.

Besser ist es, anstelle eines Arrays ein BlockingCollection zu nehmen, die mit den neuen Objekten gefüllt wird. Sobald der Füllstand 5000 Elemente beträgt, wird die Verarbeitung gestartet, die entweder nach der Verarbeitung von 5000 Elementen beendet wird, oder auch alle zwischenzeitlich hinzugefügten Elemente gleich mitverarbeitet. Die BlockingCollection hat den Vorteil, dass sie sowohl beim Hinzufügen als auch beim Entfernen von Elementen threadsicher ist.

---

--
Viele Grüsse
Peter Fleischer (MVP, Partner)
Meine Homepage mit Tipps und Tricks
Kommas richtig setzen!
Schüler sagen, Lehrer haben es gut.
Schüler, sagen Lehrer, haben es gut

Answer 4

Hallo,

dein Beispiel hat gewisse Schwächen.

1. Klasse B sollte ein Singleton oder statisch sein.
2. Die Zugriffe auf die "gemeinsamen" Daten sollten jeweils threadsafe gemacht werden

Da bleiben aber Fragen, z.B.:
Was ist eigentlich, wenn das "Wegschreiben" der 4000 Datensätze mal etwas länger dauert?
(Gibt es eigentlich einen wichtigen Grund, immer erst 4000 Einträge zu sammeln?)

In deinem Code wird nebenbei der Counter hochgezählt aber immer nur das erste Arrayelement überschrieben.

Ein grobes Schema (ungetestet) könnte so aussehen, wobei ich das Array durch eine List<string> ersetzt habe:

public class A {
  public ValueChangedEvent(object sender, MonitoredItemNotificationEventArgs e) {
    B.Instance.AddItem(e.Value);
  }
}

public class B {
  private static readonly B instance = new B();
  private static int counter = 0;
  private static long total = 0;
  private static List<string> items = new List<string>();
  private static readonly object lockobj = new object();
  private B() { }
  static B() { }
  public static B Instance => instance;
  public long Total => Interlocked.Read(ref total);

  public void AddItem(string pItem) {
    lock ( lockobj ) {
      total++;
      counter++;
      if ( 4000 < counter ) {
        // Daten wegschreiben
        foreach ( string item in items ) {
          // Hier "Insert": Ist da eventuell eine Zwischenpufferung erforderlich?
          // Solange dies hier läuft, werden alle Threads blockiert, die auf diese
          // Methode zugreifen wollen...
        }
        counter = 1;
        items.Clear();
      }
      items.Add(pItem);
    }
  }
}

Gruß, K.

Answer 5

Hallo,

dein Beispiel hat gewisse Schwächen.

1. Klasse B sollte ein Singleton oder statisch sein.
2. Die Zugriffe auf die "gemeinsamen" Daten sollten jeweils threadsafe gemacht werden

Da bleiben aber Fragen, z.B.:
Was ist eigentlich, wenn das "Wegschreiben" der 4000 Datensätze mal etwas länger dauert?
(Gibt es eigentlich einen wichtigen Grund, immer erst 4000 Einträge zu sammeln?)

In deinem Code wird nebenbei der Counter hochgezählt aber immer nur das erste Arrayelement überschrieben.

Ein grobes Schema (ungetestet) könnte so aussehen, wobei ich das Array durch eine List<string> ersetzt habe:

public class A {
  public ValueChangedEvent(object sender, MonitoredItemNotificationEventArgs e) {
    B.Instance.AddItem(e.Value);
  }
}

public class B {
  private static readonly B instance = new B();
  private static int counter = 0;
  private static long total = 0;
  private static List<string> items = new List<string>();
  private static readonly object lockobj = new object();
  private B() { }
  static B() { }
  public static B Instance => instance;
  public long Total => Interlocked.Read(ref total);

  public void AddItem(string pItem) {
    lock ( lockobj ) {
      total++;
      counter++;
      if ( 4000 < counter ) {
        // Daten wegschreiben
        foreach ( string item in items ) {
          // Hier "Insert": Ist da eventuell eine Zwischenpufferung erforderlich?
          // Solange dies hier läuft, werden alle Threads blockiert, die auf diese
          // Methode zugreifen wollen...
        }
        counter = 1;
        items.Clear();
      }
      items.Add(pItem);
    }
  }
}

Gruß, K.

Vielen Dank K.

Den kritischen Abschnitt mit lock zu Kennzeichnen hat mein Problem gelöst! :)

Answer 6

Hi,
auch Deine Idee hat Schwächen, die u.U. zu Problemen führen können.

Aller 50 ms kommt ein neues Datenobjekt. Wenn da die AddItem-Methode blockiert wird, weil das Schreiben von 5000 Datensätzen bestimmt viel länger dauert, dann können u.U. Ereignisse verloren gehen und es fehlen dann Daten.

---

--
Viele Grüsse
Peter Fleischer (MVP, Partner)
Meine Homepage mit Tipps und Tricks
Kommas richtig setzen!
Schüler sagen, Lehrer haben es gut.
Schüler, sagen Lehrer, haben es gut

Answer 7

Hi,
und was passiert, wenn das Blockieren länger als 50 ms dauert? Gehen dann Daten verloren. Ich finde die Lösung nicht gut.

---

--
Viele Grüsse
Peter Fleischer (MVP, Partner)
Meine Homepage mit Tipps und Tricks
Kommas richtig setzen!
Schüler sagen, Lehrer haben es gut.
Schüler, sagen Lehrer, haben es gut

Answer 8

Hallo Peter,

danke für die konstruktiven Hinweise. Ich glaube, auf die mögliche Problematik hatte ich aber doch mehr als deutlich hingewiesen. Meine Idee war auch nur, die Klasse B als einfaches Singleton anzulegen und threadsafe zu machen. Das war das für mich erkennbare primäre Problem. Ohne Kenntnis der weiteren Umstände (welche DB, wie angebunden, Umfang der Daten, etc.) kann ich da jedenfalls so keine klare Empfehlung aussprechen.

...

Answer 9

HI,
klar, das ohne Kenntnis weiterer Hintergründe eine passende Lösung oder Empfehlung zu geben, nicht möglich ist.

Wenn jedes INSERT 100 µs benötigt, dann brauchen die 4000 INSERT's 400 ms, was bedeutet, dass ca. 8 Signale (aller 50 ms) nicht zugreifen können und deshalb möglicherweise verloren gehen.

Ich empfehle deshalb eine BlockingCollection. Da wird nur das Einfügen und Entnehmen eines einzelnen Elementes blockiert, was dann weit unter einer Millisekunde liegt. Die Signale können damit problemlos im anderen Thread empfangen und abgelegt werden.

---

--
Viele Grüsse
Peter Fleischer (MVP, Partner)
Meine Homepage mit Tipps und Tricks
Kommas richtig setzen!
Schüler sagen, Lehrer haben es gut.
Schüler, sagen Lehrer, haben es gut