Hallo zusammen,

ich bin noch nicht ganz ThreadSave arbeite aber daran. Nach dem Lesen einiger Themen habe ich noch eine Verständnisfrage.

Schützt eine CriticalSection den kompletten Thread oder kann die CS auch auf den Zugriff auf eine Property runter gebrochen werden?
In etwa so:

zusammenfalten · markieren

Delphi-Quellcode:

type
  TMyThread = class(TThread)
  private
    FCount:Integer;
    FProp1: Real;
    FProp2: Real;

    FEventProp1: TThreadReal;
    FEventProp2: TThreadReal;

    FSectionProp1: TCriticalSection; // CriticalSection für Prop1
    FSectionProp2: TCriticalSection; // CriticalSection für Prop2

    procedure SyncEventProp1;
    procedure SyncEventProp2;

    function GetProp1: Real;
    function GetProp2: Real;

    procedure SetProp1(const Value: Real);
    procedure SetProp2(const Value: Real);
  protected
    procedure Execute; override;
  public
    property Prop1: Real read GetProp1 write SetProp1;
    property Prop2: Real read GetProp2 write SetProp2;

    property EventProp1: TThreadReal read FEventProp1 write FEventProp1;
    property EventProp2: TThreadReal read FEventProp2 write FEventProp2;

    constructor Create(sEinst: String);
    destructor Destroy; override;
  end;

implementation


constructor TMyThread.Create(sEinst: String);
begin
  inherited Create(true);

  FSectionProp1 := TCriticalSection.Create;
  FSectionProp2 := TCriticalSection.Create;
  FCount := 0;
end;

destructor TMyThread.Destroy;
begin
  FSectionProp1.Free;
  FSectionProp2.Free;

  inherited Destroy;
end;


procedure TMyThread.Execute;
begin
  inherited;

  while not Terminated do
  begin
    if Odd(FCount) then
      Prop1 := FCount
    else
      Prop2 := FCaount;
    FCount := FCount + 1;
    Sleep(50);
  end;
end;

function TMyThread.GetProp1: Real;
begin
  FSectionProp1.Acquire;
  try
    Result := FProp1;
  finally
    FSectionCount.Release;
  end;
end;

function TMyThread.GetProp1: Real;
begin
  FSectionProp1.Acquire;
  try
    Result := FProp1;
  finally
    FSectionCount.Release;
  end;
end;


procedure TMyThread.SetProp1(const Value: Real);
begin
  FSectionProp1.Acquire;
  try
    if FProp1 <> Value then // Hier bewusst FProp1 und nicht Prop1, da Property ja schon geschützt
    begin
      FProp1 := Value;
      Synchronize(SyncEventProp1); // Auch das in CS und damit Ausgabe geschützt
    end;
  finally
    FSectionProp1.Release;
  end;
end;

procedure TMyThread.SetProp2(const Value: Real);
begin
  FSectionProp2.Acquire;
  try
    if FProp2 <> Value then // Hier bewusst FProp2 und nicht Prop2, da Property ja schon geschützt
    begin
      FProp2 := Value;
      Synchronize(SyncEventProp2); // Auch das in CS und damit Ausgabe geschützt
    end;
  finally
    FSectionProp2.Release;
  end;
end;

procedure TMyThread.SyncEventProp1;
begin
  if Assigned(FEventProp1) then
    FEventConnected(FProp1); // Hier bewusst FProp1 und nicht Prop1, da Property ja schon durch SetProp1 geschützt und Sync nur von dort geschieht
end;

procedure TMyThread.SyncEventProp2;
begin
  if Assigned(FEventProp2) then
    FEventConnected(FProp2); // Hier bewusst FProp1 und nicht Prop1, da Property ja schon durch SetProp1 geschützt und Sync nur von dort geschieht
end;

end.

Im Main-Thread sollte nun Event per Syncchronize oder Zugriff auf Property möglich sein

zusammenfalten · markieren

Delphi-Quellcode:

procedure TMain.FormCreate(Sender: TObject);
begin
  MyThread.EventProp1 := EventProp1;
end;

procedure TMain.EventProp1(Value: Real);
// Ständige Aktualisierung
begin
  Label1.Caption := format('%3.2f', [Value]);
end;

procedure TMain.Button1Click((Sender: TObject);
begin
  Label2.Caption := format('%3.2f', [MyThread.Prop1]);
  Label3.Caption := format('%3.2f', [MyThread.Prop2]);
end;

Damit könnte theoretisch jemand auf Prop1 zugreifen währen der Thread in Prop2 schreibt

Ich habe hier bewusst eine Mischung aus Sychronice und Aufruf der Variablen gewählt um auch zu erfahren ob diese gemischte Verwendung Threadsafe implementiert wäre.

Bisher habe ich CS verwendet, habe aber beim erneuten Suchen gesehen, dass TMultiReadExclusiveWriteSynchronizer vielleicht besser wäre. Frage nach unterschiedlichen Blöcken gilt deshalb analog für TMultiReadExclusiveWriteSynchronizer (msProp1, msProp2)

Danke für euer Feedback
Gerd

TMultiReadExclusiveWriteSynchronizer würde ich da machen wo wirklich mehr gelesen and geschrieben wird.

Aber Vorsicht wenn du FMX einsetzt:
Unter FMX-Mobile wird das nur als normaler TMonitor implementiert (also keine MultiRead...).
Müsste ich jetzt nachsehen ob Android/iOS oder Beide.

Das sind im Endeffekt ja viele verschiedene Fragen auf einmal.

Ich denke bei den wenigstens Anwendungen ist das wirklich interessant ob im Hintergrund eine TCriticalSection, ein TMonitor, ein TMultiReadSonstwas zum Einsatz kommt. Die paar Nanosekunden sparen wir uns an dieser Stelle am besten vorerst...

Ich finde das ziemlich kompliziert gelöst. Erstens finde ich es fehl am Platz dass der Thread sich darum kümmern muss hier etwas in den Hauptthread zu pushen. Der Thread kann ein Event anbieten, das Formular registriert sich dort. Wenn das Formular es braucht dass es im Hauptthread ausgeführt wird, macht es das bitte selbst.

Wie du schon bemerkt hast ist ein Riesenproblem dass der Event-Handler ausgeführt wird während der Zugriff noch gesperrt ist. Wenn dein Formular jetzt z.B. den Wert noch einmal lesen will hängt alles - Weil deine TMyThread-Instanz hat es noch gesperrt. Folglich:

1. Zugriff sperren
2. Wert setzen, merken ob vorher anders war
3. Sperre aufheben
4. Ist der Wert vorher anders gewesen? Change-Event auslösen

Zweitens würde ich mir verschiedene Events sparen - Vielleicht ist bei mir die Welt viel einfacher, aber in 90 % der Fälle will man wirklich nur darauf reagieren dass sich etwas geändert hat und pinselt dann die Oberfläche neu. Sicher könnte man etwas Compiler-Strom sparen das alles haarklein aufzuteilen, aber im ersten Wurf braucht man das sicher nicht.

Eine kleine Änderung die ich mir noch erlaubt habe ist dass ich dem Thread nicht zwei oder mehr Werte direkt geben würde - Lager die Werte nochmal in eine einfache Struktur aus, einen Record (also Wertetyp).

Das sähe am Schluss dann so aus:

zusammenfalten · markieren

Delphi-Quellcode:

unit Unit2;

interface uses
   System.Classes,
   System.SyncObjs;

type
   TData = record
      Prop1:   Real;
      Prop2:   Real;
    end;

   TDataChangeEventHandler = procedure(const data: TData) of object;

   TMyThread = class(TThread)
      private var
         handler:   TDataChangeEventHandler;
         FCount:      Integer;
      private
         procedure incCounter(); inline;
      protected var
         data: TData;
         mutex: TCriticalSection;
      protected
         procedure Execute(); override;
         procedure setProp1(const value: Real);
         procedure setProp2(const value: Real);
         procedure invokeChangeEvent(); inline;
      public
         constructor Create(CreateSuspended: Boolean);
         destructor Destroy(); override;

         procedure setOnChange(const handler: TDataChangeEventHandler);
         function getOnChange(): TDataChangeEventHandler;
         function getData(): TData;
   end;

implementation uses System.SysUtils;

{ TMyThread }

constructor TMyThread.Create(CreateSuspended: Boolean);
begin
   inherited Create(CreateSuspended);
   mutex := TCriticalSection.Create();
end;

destructor TMyThread.Destroy();
begin
   mutex.Free();
   inherited;
end;

procedure TMyThread.Execute();
begin
   while (not Terminated) do
      begin
         if Odd(FCount) then
            setProp1(FCount)
         else
            setProp2(FCount);

         incCounter();
         TThread.Sleep(50);
      end;
end;

function TMyThread.getData(): TData;
begin
   mutex.Acquire();
   try
      Result := data;
   finally
      mutex.Release();
    end;
end;

function TMyThread.getOnChange(): TDataChangeEventHandler;
begin
   mutex.Acquire();
   try
      Result := handler;
   finally
      mutex.Release();
    end;
end;

procedure TMyThread.incCounter();
begin
   if (FCount < Integer.MaxValue) then
      Inc(FCount)
   else
      FCount := Integer.MinValue;
end;

procedure TMyThread.invokeChangeEvent();
var
   _handler: TDataChangeEventHandler;
begin
   _handler := getOnChange();
   if Assigned(_handler) then
      _handler(data);
end;

procedure TMyThread.setOnChange(const handler: TDataChangeEventHandler);
begin
   mutex.Acquire();
   try
      self.handler := handler;
   finally
      mutex.Release();
   end;
end;

procedure TMyThread.setProp1(const value: Real);
var
   hasChanged: Boolean;
begin
   mutex.Acquire();
   try
      hasChanged := (data.Prop1 <> value);
      if hasChanged then
         data.Prop1 := value;
   finally
      mutex.Release();
   end;

   if hasChanged then
      invokeChangeEvent();
end;

procedure TMyThread.setProp2(const value: Real);
var
   hasChanged: Boolean;
begin
   mutex.Acquire();
   try
      hasChanged := (data.Prop2 <> value);
      if hasChanged then
         data.Prop2 := value;
   finally
      mutex.Release();
   end;

   if hasChanged then
      invokeChangeEvent();
end;

end.

Vielen Dank für das schnelle Feedback. Ein paar Dinge habe ich schon umgesetzt.

Vielleicht kurz beschrieben, an was ich gerade dran bin.
Universelle ComPort-Ansteuerung die je nach Gerät ein oder zwei Befehle automatisch (pollend) schickt und zusätzlich die Möglichkeit hat, zusätzliche Befehle vom Main entgegen zu nehmen, zu senden und Antwort zurück zugeben.
Da die Befehle immer seriell gesendet werden, kam das mit zwei Events für Prop1 (z.B. Position) und Prop2 (z.B. Speed) auf.
Z.B. wird in einer Anwendung aufgrund der Position ein Motor abgeschalten. => Position wird öfter abgefragt als Speed und im Event des Mains entsprechend ausgewertet. Ist aber kein Killer für Rückgabe als Record.

Hier kurz zusammengefasst wie ich es verstanden habe

• Eigene CriticalSection machen keinen Sinn
• Zeiteinsprungen zwischen CriticalSection und TMultiReadExclusiveWriteSynchronizer vernachlässigbar
• Werte in Klasse oder Record zusammenfassen kann Handling vereinfachen da nur ein Event-Handler im Main nötig

Die Bemerkung

Zitat:

Wie du schon bemerkt hast ist ein Riesenproblem dass der Event-Handler ausgeführt wird während der Zugriff noch gesperrt ist. Wenn dein Formular jetzt z.B. den Wert noch einmal lesen will hängt alles - Weil deine TMyThread-Instanz hat es noch gesperrt.

habe ich nicht ganz verstanden. Falls sich der auf meinen Zusatz bezieht

Zitat:

Damit könnte theoretisch jemand auf Prop1 zugreifen währen der Thread in Prop2 schreibt

hatte ich den als Chance gesehen. Ich dachte durch die zwei Sections ist es möglich, dass der Main auf Prop1 zugreigt und der Thread in Prop2 schreibt ohne dass sich die ins Gehege kommen. Sehe ich das falsch?
Wo ist bei mir die Sicherheitslücke?

Und noch eine Frage hierzu

markieren

Delphi-Quellcode:

procedure TMyThread.invokeChangeEvent();
var
   _handler: TDataChangeEventHandler;
begin
   _handler := getOnChange();
   if Assigned(_handler) then
      _handler(data);
end;

Warum der Zwischenschritt über _handler? Warum nicht so?

markieren

Delphi-Quellcode:

procedure TMyThread.invokeChangeEvent();
begin
   if Assigned(getOnChange) then
      getOnChange(data);
end;

Gewohnheit oder begründet?

Weil es hochtheoretisch sein könnte dass jemand in der Zwischenzeit zwischen "if" und "then" aus einem anderen Thread setOnChange(..) aufruft und einen anderen Handler (oder garkeinen) setzt. Dann würdest du im "then"-Teil einen leeren Funktionszeiger aufrufen wollen und es gäbe eine Zugriffsverletzung.

Könnte man das OnChange nicht mittendrin ändern (z.B. würde es nur einmal im Konstruktor des Threads übergeben werden) müsste man sich davor nicht fürchten und man bräuchte es nicht und könnte einfach sagen

markieren

Delphi-Quellcode:

   if Assigned(getOnChange) then
      getOnChange(data);

Zitat von norwegen60:

habe ich nicht ganz verstanden. (...)
Wo ist bei mir die Sicherheitslücke?

Was ich meinte: Angenommen wir nutzen unsere Sperre nicht nur für das Setzen des Wertes sondern auch führ das Ausführen der Events, also z.B. der Aktualisierung deines Formulars. Aus

markieren

Delphi-Quellcode:

Sperre.Betreten()
try
   wert = neuer wert
finally
   Sperre.Verlassen()
end;

wird also

markieren

Delphi-Quellcode:

Sperre.Betreten()
try
   wert = neuer wert
   Wenn der Wert sich geändert hat:
      Führe OnChange aus
finally
   Sperre.Verlassen()
end;

Und dein "OnChange" ist gleichzeitig auch noch dass wir mit TThread.Synchronize(..) hantieren (davon hatte ich ja stark abgeraten).

Dann kann es sein dass du in deiner Ereignisbehandlung (also auf deinem Formular) einen Getter oder Setter vom Thread aufrufst. Niemand kann dich daran hindern. Nur was passiert dann? Er will wieder sperren. Und darauf wird er ewig warten, denn der Thread hält es noch gesperrt und wartet darauf dass der TThread.Synchronize(..) fertig wird. Der wird aber nie fertig, da er auf die Sperre wartet. Eine Verklemmung auf ewig.

Das meinte ich.

Weiterhin ist es denke ich unnötig dass der Thread so lange pausiert und wartet bis jemand darauf reagiert hat. Deine serielle Kommunikation würde nun so lange stehen bis das Gepinsel auf dem Formular fertig ist.

OK, weitestgehend verstanden (Hoffe ich)

Rein theoretisch und vor allem für mein Verständnis:

1. TMultiReadExclusiveWriteSynchronizer würde das Problem beseitigen da hier mehrer Lese-Zugriffe möglich wären und diese Werte nie vom Main gesetted werden?
2. Eigene Section für jedes Property würden das Problem verringern, da es nur beim Zugriff auf dieselbe Prop klemmen würde?
3. Hängt der Thread nicht auch bei deinem invokeChangeEvent bis sich die GUI gepinselt hat?
4. Und müsste das, wenn separat verschickt, nicht auch in eine CS da ja sonst Main und Thread auf data zugreifen könnten?

markieren

Delphi-Quellcode:

procedure TMyThread.invokeChangeEvent();
var
   _handler: TDataChangeEventHandler;
begin
   _handler := getOnChange();
   if Assigned(_handler) then
      _handler(data);
end;

1. Ich denke der TMultiReadExclusiveWriteSynchronizer ist eine angemessene Lösung wenn man wirklich die Aufgabenstellung hat wenn ein Wert über mehrere Threads abgesichert werden muss und hierbei sehr oft gelesen, aber nur selten geschrieben wird. Klar war dein Beispiel konstruiert, aber das Sleep(50) ließ jetzt auf eine eher häufige Änderung schließen. Ein 100%iges Versprechen Deadlocks zu vermeiden kann dir auch ein TMultiReadExclusiveWriteSynchronizer nicht geben, er bringt nur in speziellen Anwendungsfällen wohl bessere Performance.
2. Ja, aber ob der exorbitant höhere Schreibaufwand und erhöhte Komplexität es wert ist dass das ursprüngliche Problem nur etwas verringert wurde?

Zu Punkt 3 & 4: Da hast du Recht. Ich hatte nur das Lesen auf dem Schirm, aber hochtheoretisch kann jemand setProp1(..) aufrufen, und vor der letzten Zeile wird der Thread angehalten und von einem anderen wird setProp2(..) aufgerufen. Der läuft komplett durch und später wird setProp1(..) noch ordentlich zu Ende gebracht. Keine Datenkorruption, aber das OnChange-Event wurde dann einfach zwei mal mit den neuen Werten aufgerufen.

Zitat von Uwe Raabe:

Zitat von Stevie:

Seit der PPL denken plötzlich alle, Multithreading und parallel programming sei nen Kindergeburtstag

Aus den Slides meines Vortrags auf den Delphi-Tagen 2013:

Zitat von Danny Thorpe:

New programmers are drawn to multithreading like moths to flame, with similar results.

Danke noch für das Feedback. Dass TMultiReadExclusiveWriteSynchronizer nur bei vielen Lesevorgängen wirklich Sinn macht, war mir bewusst.
Der kurze Sleep kommt daher, dass eingehende Befehle schnell abgearbeitet werden sollen, was aber auch über Event lösbar wäre

Ich habe jetzt meine Anwendung so, dass sie grundsätzlich funktioniert.
Wenn ich aber gewisse Funktionen aktiviere, bekomme ich Deadlocks und weiß nicht so genau warum.
Deshalb habe ich doch noch mal was grundsätzliches.
Zum leichteren Nachvollziehen habe ich ein ein kleines Beispiel konstruiert:

zusammenfalten · markieren

Delphi-Quellcode:

unit uThrTest;

interface

uses
  Classes,
  SyncObjs;

type
  TThreadString = procedure(Value: String) of object;

  TthrTest = class(TThread)
  private
    FWert: String;
    FListe: TStringList;

    FSyncWert: TThreadString;

    procedure SyncEventWert;

    function GetWert: String;
    procedure SetWert(const Value: String);
  protected
    FSection: TCriticalSection;

    procedure Execute; override;
  public
    property Wert: String read GetWert write SetWert;

    property SyncWert: TThreadString read FSyncWert write FSyncWert;

    constructor Create(CreateSuspended: Boolean);
    destructor Destroy; override;

    procedure SendToListe(sValue: String);
  end;

implementation

//******************************************************************************************************************************************
{ TthrProperty }
//******************************************************************************************************************************************

constructor TthrTest.Create(CreateSuspended: Boolean);
//******************************************************************************************************************************************
begin
  inherited Create(CreateSuspended);

  FSection := TCriticalSection.Create;
  Wert := '';
  FListe := TStringList.Create;
end;

destructor TthrTest.Destroy;
//******************************************************************************************************************************************
begin
  Terminate;
  WaitFor;
  FListe.Free;
  FSection.Free;
end;

procedure TthrTest.Execute;
//******************************************************************************************************************************************
begin
  inherited;

  while not Terminated do
  begin
    FSection.Acquire; // Zugriff auf FListe schützen
    try
      if FListe.Count > 0 then
      begin
        Wert := FListe[0];
        FListe.Delete(0);
      end;
    finally
      FSection.Release; // Zugriff auf FListe wieder freigeben
    end;
    TThread.Sleep(300);
  end;
end;

procedure TthrTest.SendToListe(sValue: String);
//******************************************************************************************************************************************
begin
  if (sValue <> '') then
  begin
    FSection.Acquire; // Zugriff auf FListe schützen
    try
      FListe.Add(sValue);
    finally
      FSection.Release; // Zugriff auf FListe wieder freigeben
    end;
  end;
end;

function TthrTest.GetWert: String;
//******************************************************************************************************************************************
begin
  FSection.Acquire; // Zugriff auf FWert schützen
  try
    Result := FWert;
  finally
    FSection.Release; // Zugriff auf FWert wieder freigeben
  end;
end;

procedure TthrTest.SetWert(const Value: String);
//******************************************************************************************************************************************
var
  bChanged: Boolean;

begin
  FSection.Acquire; // Zugriff auf FWert schützen
  try
    bChanged := (FWert <> Value);
    if bChanged then
      FWert := Value;
  finally
    FSection.Release; // Zugriff auf FWert wieder freigeben
  end;

  // Nicht innerhalb der CriticalSection da Main sonst blockieren könnte
  if bChanged then
    Synchronize(SyncEventWert);
end;

procedure TthrTest.SyncEventWert;
//******************************************************************************************************************************************
begin
  if Assigned(FSyncWert) then
    FSyncWert(FWert); // Zugriff auf FWert da durch Synchronize schon Schutz erzeugt wird
end;

end.

Das Ziel ist:

• Über den Main-Thread soll eine Liste gefüllt werden. Das kann sehr schnell passieren (In meinem Fall Kommandos an ein Gerät. Die Liste dient als Puffer)
• Die Antwort wird in FWert geschrieben. Der Wert wird bei jeder Änderung mit dem Main synchronisiert.
• Zusätzlich soll der Main die Möglichkeit haben schreibend und lesend auf FWert zuzugreifen

Meine Fragen:

1. Ist der Zugriff auf die Liste sauber geschützt?
2. Ist der Zugriff auf FWert sauber implementiert?
3. Welche Möglichkeiten gibt es, dass der SyncWert zum Main zu keiner Verzögerung führt auch wenn die Abarbeitung im Main länger dauert? (Im Moment löse ich es, dass FWert auf der Main-Seite in eine Liste geschrieben wird, und die abgearbeitet wird)
4. Ist es richtig, dass in SyncEventWert nur auf FWert zugegriffen werden darf (nicht auf Wert)? Ich meine durch den Aufruf von Synchronize(SyncEventWert) wird schon ein Schutz erzeugt und wenn ich jetzt auf Wert zugreife, kommt der zusätzliche Section-Schutz um GetWert zum Tragen.
5. Wird bei mehrfachem Aufruf von TSection.Acquire erkannt, wenn der aus demselben Thread geschieht oder führt auch das zu einer Blockade?
6. Ist an meinem Code sonst was falsch?

Zum einfacheren anschauen habe ich den kompletten Beispiel-Code incl. Main-Form als Zip angehängt.

• Durch Klick auf [Write into List] werden Einträge in die Liste geschrieben. Durch schnelles Klicken sieht man, dass Liste langsamer abgearbeitet wird.
• Durch Klick auf [Write into Wert] wird direkt in FWert geschrieben und auch sofort wieder zurück gegeben
• Durch Klick auf [Read Wert] wird FWert direkt gelesen

Vielen Dank
Gerd