CriticalSection und Threads

[Sir Rufo]

Aus aktuellem Anlass und weil mit das Handling der CriticalSections gerade beim Vererben von Threads etwas genervt hat hier mal ein smarter Ansatz:

Wenn in einem Thread eine CriticalSection benötigt wird (Schutz der Datenzugriffe durch unterschiedliche Threads), dann sollte die CriticalSection folgendermassen erzeugt werden:

Delphi-Quellcode:

unit uThreadA;

interface

uses
  Classes, SyncObjs;

type
  TMyThreadA = class( TThread )
  strict private
    _CS : TCriticalSection;
  protected
    function CS : TCriticalSection;
  public
    constructor Create( CreateSuspended : Boolean );
    destructor Destroy; override;
  end;

implementation

{ TMyThreadA }

function TMyThreadA.CS : TCriticalSection;
begin
  if not Assigned( _CS )
  then
    _CS := TCriticalSection.Create;

  Result := _CS;
end;

constructor TMyThreadA.Create( CreateSuspended : Boolean );
begin
  CS.Enter;
  try
    inherited;

  finally
    CS.Leave;
  end;
end;

destructor TMyThreadA.Destroy;
begin
  CS.Enter;
  try

    inherited;
  finally
    CS.Leave;
    // Eigentlich FreeAndNil( _CS ) aber wenn man sich die SysUtils sparen kann ;o)
    _CS.Free;
    _CS := nil;
  end;
end;

end.

Die CriticalSection wird noch vor der eigentlichen Objekt-Instanz erzeugt und schützt auch schon das Erzeugen dieser Instanz.
Die CriticalSection wird nach der Freigabe der Objekt-Instanz freigegeben und die Freigabe der Objekt-Instanz wird von der CriticalSection noch geschützt.

So weit, so gut ...

Aber was passiert, wenn man diese Thread-Klasse vererben möchte?

Delphi-Quellcode:

unit uThreadB;

interface

uses
  Classes, SyncObjs, uThreadA;

type
  TMyThreadB = class( TMyThreadA )
  public
    constructor Create( CreateSuspended : Boolean );
    destructor Destroy; override;
  end;

implementation

{ TMyThreadB }

constructor TMyThreadB.Create( CreateSuspended : Boolean );
begin
  CS.Enter;
  try
    inherited;

  finally
    CS.Leave;
  end;
end;

destructor TMyThreadB.Destroy;
begin
  CS.Enter;
  try

    // inherited
    // darf hier nicht stehen, denn sonst wird ja die CS freigegeben 
    // und im finally-Teil gibt es eine Exception
  finally
    CS.Leave;
  end;

  inherited;
end;

end.

Eine Alternative wäre, in jedem Thread (auch den abgeleiteten) eine neue CS einzuführen.

Warum kann die CS denn kein Interface sein? ;o)

Dann könnte man das sehr hübsch so machen:

Delphi-Quellcode:

unit uThreadA;

interface

uses
  Classes, SyncObjs;

type
  TMyThreadA = class( TCSThread ) // eine neue Basisklasse Thread mit CS
  public
    constructor Create( CreateSuspended : Boolean );
    destructor Destroy; override;
  end;

implementation

{ TMyThreadA }

constructor TMyThreadA.Create( CreateSuspended : Boolean );
begin
  CS.Enter;
  try
    inherited;

  finally
    CS.Leave;
  end;
end;

destructor TMyThreadA.Destroy;
begin
  CS.Enter;
  try

    inherited;
  finally
    CS.Leave;
  end;
end;

end.

und der davon abgeleitete Thread wird auf die gleiche Weise implementiert, denn nun verflüchtigt sich die CriticalSection dann, wenn diese nicht mehr benötigt wird.

Delphi-Quellcode:

unit uThreadB;

interface

uses
  Classes, SyncObjs, uThreadA;

type
  TMyThreadB = class( TMyThreadA )
  public
    constructor Create( CreateSuspended : Boolean );
    destructor Destroy; override;
  end;

implementation

{ TMyThreadB }

constructor TMyThreadB.Create( CreateSuspended : Boolean );
begin
  CS.Enter;
  try
    inherited;

  finally
    CS.Leave;
  end;
end;

destructor TMyThreadB.Destroy;
begin
  CS.Enter;
  try

    inherited
  finally
    CS.Leave;
  end;
end;

end.

Und hier das Interface mit der Thread-Klasse:

Delphi-Quellcode:

unit uCSObjects;

interface

uses
  Classes,
  SyncObjs;

type
  ICriticalSection = interface
    ['{DE0BF9E0-92C0-424B-A70F-5C58CD412C1A}']
    procedure Enter;
    function TryEnter : Boolean;
    procedure Leave;
  end;

  TInterfacedCriticalSection = class( TInterfacedObject, ICriticalSection )
  strict private
    _CS : TCriticalSection;
  protected
    procedure Enter;
    function TryEnter : Boolean;
    procedure Leave;
  public
    constructor Create;
    destructor Destroy; override;
  end;

  TCSThread = class( TThread )
  strict private
    _CS : ICriticalSection;
  protected
    function CS : ICriticalSection;
  end;

implementation

{ TInterfacedCriticalSection }

constructor TInterfacedCriticalSection.Create;
begin
  inherited;
  _CS := TCriticalSection.Create;
end;

destructor TInterfacedCriticalSection.Destroy;
begin
  _CS.Free;
  _CS := nil;
  inherited;
end;

procedure TInterfacedCriticalSection.Enter;
begin
  _CS.Enter;
end;

procedure TInterfacedCriticalSection.Leave;
begin
  _CS.Leave;
end;

function TInterfacedCriticalSection.TryEnter : Boolean;
begin
  Result := _CS.TryEnter;
end;

{ TCSThread }

function TCSThread.CS : ICriticalSection;
begin
  if not Assigned( _CS )
  then
    _CS  := TInterfacedCriticalSection.Create;
  Result := _CS;
end;

end.

[himitsu]

Zitat:

Delphi-Quellcode:

function TMyThreadA.CS : TCriticalSection;
begin
  if not Assigned( _CS )
  then
    _CS := TCriticalSection.Create;

  Result := _CS;
end;

CS ist nicht threadsave, denn wenn zwei Threads diese Funktion zum ersten Mal (wärend _CS noch nil ist) gleichzeitig aufrufen, dann gibt es da eventuell zwei Problemchen.
> es können mehrere TCriticalSection erstellt werden
= Speicherleck
= die direkten nachfolgenden Aufrufe wären nicht geschützt, da sie CriticalSections nutzen würden.
= es würde zu einem Deadlock kommen *1

1) CS.Enter würde eventuell, beim Erstellen, unterschiedliche CriticalSections,
aber CS.Leave würde richtig funktionieren und es würden dann einige Leaves auf eine falsche CS zeigen, was die Sperr-Zählung schrottet.


Lösung:
Erstell die CS schon im Constructor, denn der ist immer threadsave, Dank des Highlanderprinzips.
Wenn sie unbeding erst in dieser Funktion erstellt werden soll, dann sag Bescheid und gib mir ein Minütchen.

[Sir Rufo]

Der Einwand ist berechtigt, jedoch das Problem ist eher theoretisch, denn eine CS für eine Instanz sollte so implementiert werden:

Delphi-Quellcode:

constructor TMyThreadA.Create( CreateSuspended : Boolean );
begin
  CS.Enter; // erster Aufruf für die neue Instanz
  try
    inherited; // ab jetzt gibt es eine Instanz

  finally
    CS.Leave;
  end;
end;

Ein Durcheinander kann somit nur dann auftreten, wenn ich es schaffe von 2 unterschiedlichen Threads die gleiche Objekt-Instanz zu erzeugen.
Und das ist meines Wissens nicht möglich.

Um auch diesen theoretischen Fall abzusichern, könnte man noch eine CS für die Unit verwenden, die dieses dann regeln würde.

Delphi-Quellcode:

unit uCSObjects;

interface

[...]

implementation

var
  __CS : ICriticalSection; // Eine CriticalSection für diese Unit

{ TInterfacedCriticalSection }

[...]

{ TCSThread }

function TCSThread.CS : ICriticalSection;
begin
  __CS.Enter;
  try

    if not Assigned( _CS )
    then
      _CS := TInterfacedCriticalSection.Create;

  finally
    __CS.Leave;
  end;

  Result := _CS;
end;

initialization

__CS := TInterfacedCriticalSection.Create;

end.

[r2c2]

Warum haben deine Konstruktoren denn überhaupt Seiteneffekte? Denn nur dann braucht man ja die CriticalSections.

mfg

Christian

[Furtbichler]

Ich verstehe das Problem nicht. Ich erzeuge meine privaten Felder, also auch eine CS, im Konstruktor und gebe sie im Destruktor wieder frei.

Weiterhin vestehe ich nicht, weshalb ich sowohl Konstruktor als auch Destruktor gegen parallele Aufrufe schützen soll. Versucht man hier, ein Problem zu lösen, das es bei halbwegs brauchbarer Programmierung gar nicht geben kann? Konstruktoren sind singulär, aber Destruktoren könnten gleichzeitig aufgerufen werden. Aber nur, wenn ich schlampig programmiere.

Bei mir gibt es klare Zuständigkeiten. Wer ein Objekt instantiiert, gibt es auch wieder frei. Baste. Natürlich schütze ich Daten gegen simultane Zugriffe, aber speziell Threads instantiiere ich an einer Stelle und gebe sie dort auch wieder frei. Singulär und wohldefiniert.

Threads gebe ich dann frei, wenn alle Verwender des Threads entweder selber alle tot sind oder Bescheid wissen, das der freizugebende Thread seine Arbeit beendet hat.

Die einzigen Objekte, die keine direkte Zuständigkeit haben, sind Singletons. Diese werden -wupps- irgendwann (z.B. on demand) erzeugt und schweben frei im Raum. Hier muss man wirklich etwas aufpassen. Allerdings existiert hier auch nur ein einziger Destroy-Aufruf. Zu doppelten Aufrufen kann es dann gar nicht kommen, höchstens zu Verwendung, nachdem das Singleton freigegeben wurde.

[Sir Rufo]

Zitat:

Zitat von r2c2 (Beitrag 1137938)

Warum haben deine Konstruktoren denn überhaupt Seiteneffekte? Denn nur dann braucht man ja die CriticalSections.

mfg

Christian

Wenn ich in meiner Anwendung keine Threads benutze, dann kann ich mir das sparen - richtig.

Aber wenn ich Thread-Safe Objekte benötige, dann könnten auch die Konstruktoren Seiteneffekte haben, je nachdem was ich im Konstruktor mache.
z.B. könnte ich dort die Instanz an einen bestehen Thread übergeben
Wäre der Konstruktor also nicht geschützt, dann würde es sofort knallen, wenn ich mit dem Konstruktor noch nicht fertig bin und der Thread schon auf meine Instanz zugreift.
Ich stelle einfach damit sicher, das es nicht knallen kann.

Es geht also darum, wie ich Objekte Thread-Safe bekomme - und ich möchte meine Objekte generell Thread-Safe haben und nicht nur unter ganz bestimmten Bedingungen.

Warum sollte man eine Klasse so deklarieren?

Delphi-Quellcode:

TFoo = class // Ableitung von TObject
public
  constructor Create;
end;

constructor TFoo.Create;
begin
  inherited; // wofür sollte das gut sein, denn TObject.Create beinhaltet nichts 

  // irgendwas

end;

Ganz einfach, im Moment beinhaltet der Konstruktor von TObject keinen Code, aber er könnte irgendwann mal Code haben und ich bin auch in Zukunft safe.

[himitsu]

Delphi-Quellcode:

function TMyThreadA.CS: TCriticalSection;
var
  Comp: TCriticalSection;
begin
  Result := _CS;
  if not Assigned(Result) then begin
    Result := TCriticalSection.Create;
    Comp   := InterlockedCompareExchangePointer(Pointer(_CS), Pointer(Result), nil);
    if Assigned(Comp) then begin
      Result.Free;
      Result := Comp;
    end;
  end;
end;

Braucht keine zusätzliche CS.

Nur im Destructor, vor dem Freigeben der CS, diese dort besser nochmal mal sperren.

Delphi-Quellcode:

CS.Enter;
CS.Free;

Nicht daß sie noch in Benutzung ist, beim Freigeben.

[r2c2]

Zitat:

Zitat von Sir Rufo (Beitrag 1137941)

Wenn ich in meiner Anwendung keine Threads benutze, dann kann ich mir das sparen - richtig.

Das ist klar; war aber nicht meine Aussage. Ich denke, man sollte Seiteneffekte in Konstruktoren eh möglichst vermeiden. Manchmal sind sie notwendig, aber häufig gibt es bessere Möglichkeiten. Furtbichler hat das ganze nochmal ausführlicher beschrieben: Wenn man sauber programmiert, braucht man sowas nur in Ausnahmefällen.

Zudem sind es ja normalerweise nicht die Threads, die Thread-Safe sein müssen, sondern die Objekte, auf die die Threads zugreifen.

Zitat:

z.B. könnte ich dort die Instanz an einen bestehen Thread übergeben

Einen Thread an einen anderen Thread übergeben? Ich versteh nicht, was du meinst. Aber klar, es gibt Fälle, in denen man Seiteneffekte braucht. Das will ich nicht bestreiten.

Zitat:

Wäre der Konstruktor also nicht geschützt, dann würde es sofort knallen.

Schön wärs. Das fiese ist ja, dass es oft dennoch irgendwie tut und irgendwann zu sehr lustigen Effekten kommt.

Zitat:

Es geht also darum, wie ich Objekte Thread-Safe bekomme - und ich möchte meine Objekte generell Thread-Safe haben und nicht nur unter ganz bestimmten Bedingungen.

Vielleiht hab ich dich auch falsch verstanden. Willst du eine Thread-Safe Thread-Klasse haben (wie dein Code suggeriert) oder willst du andere Objekte Thread-Safe machen (und das aber richtig) oder willst du einfach immer alles Thread-Safe machen?

Zitat:

Warum sollte man eine Klasse so deklarieren?

Delphi-Quellcode:

TFoo = class // Ableitung von TObject
public
  constructor Create;
end;

constructor TFoo.Create;
begin
  inherited; // wofür sollte das gut sein, denn TObject.Create beinhaltet nichts 

  // irgendwas

end;

Ganz einfach, im Moment beinhaltet der Konstruktor von TObject keinen Code, aber er könnte irgendwann mal Code haben und ich bin auch in Zukunft safe.

Da geh ich mit dir. Das hat mit der gegebenen Problematik aber wenig zu tun. Oder erwartest du, dass in TObject Seiteneffekte auftreten? Dann hättest du eh noch *ganz* andere Probleme. Auf so eine bescheuerte Idee wird Embarcadero zum Glück wohl eher nicht kommen. Das musst du nicht absichern.

mfg

Christian

[Sir Rufo]

Zitat:

Zitat von r2c2 (Beitrag 1137943)

Zitat:

Wäre der Konstruktor also nicht geschützt, dann würde es sofort knallen.

Schön wärs. Das fiese ist ja, dass es oft dennoch irgendwie tut und irgendwann zu sehr lustigen Effekten kommt.

Gut vielleicht hätte ich schreiben sollen "könnte" - ich gehe aber immer davon aus, dass es knallt und
gerade das kann ich damit ja verhindern ...

Zitat:

Zitat von r2c2 (Beitrag 1137943)

Zitat:

Es geht also darum, wie ich Objekte Thread-Safe bekomme - und ich möchte meine Objekte generell Thread-Safe haben und nicht nur unter ganz bestimmten Bedingungen.

Vielleiht hab ich dich auch falsch verstanden. Willst du eine Thread-Safe Thread-Klasse haben (wie dein Code suggeriert) oder willst du andere Objekte Thread-Safe machen (und das aber richtig) oder willst du einfach immer alles Thread-Safe machen?

Was ist der Unterschied? Ein TThread ist auch einfach nur ein Objekt ...
Benutze ich hier aber beispielhaft um das zu erläutern, denn Thread-Safe benötige ich ja nur im Zusammenhang mit Threads.
Aber welches Objekt man auf die skizzierte Art und Weise schützt ist egal.

[r2c2]

Zitat:

Zitat von Sir Rufo (Beitrag 1137947)

Zitat:

Vielleiht hab ich dich auch falsch verstanden. Willst du eine Thread-Safe Thread-Klasse haben (wie dein Code suggeriert) oder willst du andere Objekte Thread-Safe machen (und das aber richtig) oder willst du einfach immer alles Thread-Safe machen?

Was ist der Unterschied? Ein TThread ist auch einfach nur ein Objekt ...
Benutze ich hier aber beispielhaft um das zu erläutern, denn Thread-Safe benötige ich ja nur im Zusammenhang mit Threads.
Aber welches Objekt man auf die skizzierte Art und Weise schützt ist egal.

So langsam versteh ich, was du meinst. War nur ein bisschen unintuitiv für mich, dass du nen Thread gleichzeitig als Kontext und Beispiel verwendet hast.

Was du hier also beschriebst, ist eine Anwendung des RAII-Idioms zum Verwalten der CS-Instanzen. OK. Bleibt noch das Problem, das himitsu genannt hat, aber dafür hat er ja auch schon ne Lösung gepostet. Das ist soweit OK und auch ein interessanter Ansatz.

Trotzdem fällt mir kein sinnvoller Fall ein, in dem das explizite Schützen des Konstruktors helfen sollte. Denn wie du selbst sagst, dazu müssten zwei Threads gleichzeitig das selbe (und nicht nur das gleiche) Objekt erzeugen, was ausgeschlossen ist. Denn, selbst wenn du Seiteneffekte im Konstruktor hast, müsste das Ziel der Seiteneffekte Thread-Safe sein und nicht der Konstruktor selbst. Ansonsten hindert dich ja nichts daran das genannte "Ziel" gleichzeitig aus nem anderen Thread zu bearbeiten. Daneben hilft dein Ansatz auch nicht, wenn du gleichzeitig zwei verschiedene Instanzen der selben Klasse erzeugst, wenn im Konstruktor eine nicht thread-safer Seiteneffekt ausgeführt wird, da es sich m zwei unterschiedliche CriticalSections handelt.

mfg

Christian

[Furtbichler]

Kann mir einer mal ein Beispiel nennen, wo ein Konstruktor knallen kann, weil 'er noch nicht fertig ist'?

Wenn ich schreibe:

Delphi-Quellcode:

Foo := TMyThread.Create;

Hat Foo erst dann einen Wert, wenn der Konstruktor fertig ist.

Da es eine Grundregel gibt, die besagt, das Konstruktoren keinen logischen Code beinhalten sollen, kann ich mir irgendwelche komplex konstruierten Gebilde sparen, wo ich vielleicht doch auf ein Objekt zugreife, dessen Konstruktor noch nicht abgearbeitet ist, z.B.:

Delphi-Quellcode:

Constructor TMyThread.Create (aSomeObject : TSomeObject);
Begin
  ...
  aSomeObject.CallMe(Self);
...
End;

Procedure TSomeObject.CallMe (aThread : TMyThread);
Begin
  aThread.Bang();
End;

So, dagegen kann man sich also absichern, ja? Wozu? Sowas macht man einfach nicht:

Delphi-Quellcode:

Constructor TMyThread.Create (aSomeObject : TSomeObject);
Begin
  ...
  FSomeObject := aSomeObject;
...
End;

Procedure TMyThread.Initialize();
Begin
  FSomeObject.CallMe(Self);
End;

Procedure TSomeObject.CallMe (aThread : TMyThread);
Begin
  aThread.Bang();
End;

Der Verwender des TMyThread muss eben Initialize() aufrufen. Und das kann er erst, wenn die Instanz (vollständig) instantiiert wurde.

Ich bleib dabei: Diese Gedankenspiele sind nett, aber lenken vom eigentlichen Problem ab. Sofern man das Gedankenspiel nicht als solches auffässt.

Wie wäre es mit Überlegungen, die Endlosschleifen erkennen?

Delphi-Quellcode:

While True do;

Obwohl ich ein System kenne, das versucht, soetwas zu erkennen (so ein Schwachsinn).

[r2c2]

Zitat:

Zitat von Furtbichler (Beitrag 1137955)

Kann mir einer mal ein Beispiel nennen, wo ein Konstruktor knallen kann, weil 'er noch nicht fertig ist'?

Du kannst sowas haben, wenn du im Konstruktor z.B. auf nen Parameter zugreifst und Self übergibst. In der VCL wird beispielsweise im Konstruktor das zu erzeugende Objekt dem Owner übergeben. So etwas wäre ein Fall, der Probleme machen kann. Aus diesem und ähnlichen Gründen ist die VCL nicht thread-safe.

Allerdings: Der oben beschriebene Ansatz löst dieses Problem, wie ich oben schon geschrieben habe, gerade *nicht*. Und ein paar andere Fälle löst er auch nicht. Im Grunde genommen gebe ich dir also damit recht, dass die CS im Konstruktor nichts bringt.

mfg

Christian

[mjustin]

Zitat:

Zitat von Sir Rufo (Beitrag 1137928)

Und hier das Interface mit der Thread-Klasse:

Wäre das nicht sehr viel einfacher mit System.TMonitor implementiert?

Jedes Objekt "enthält" ab Delphi 2009 ein "Lock", das wie eine Critical Section benutzt werden kann (und noch mehr kann wie z.B. PulseAll und Wait) - und es verfällt automatisch mit Ablauf der Objekt-Lebenszeit.

Dass Threads von außen nach ihrem Start aufgerufen werden erscheint mir ansonsten auch etwas ungewöhnlich - in welchem Status der Thread gerade ist, wenn man eine seiner Operationen aufruft, ist unbestimmt.

[Sir Rufo]

Zitat:

Zitat von r2c2 (Beitrag 1137950)

Trotzdem fällt mir kein sinnvoller Fall ein, in dem das explizite Schützen des Konstruktors helfen sollte. Denn wie du selbst sagst, dazu müssten zwei Threads gleichzeitig das selbe (und nicht nur das gleiche) Objekt erzeugen, was ausgeschlossen ist.

Das mit den 2 Threads, die die gleiche Objekt-Instanz erzeugen, war als Antwort auf himitsu's Anmerkung, dass die Erzeugung der CS-Instanz nicht Thread-Safe ist und ich damit eigentlich nur aufzeigen wollte, dass dieser Fall nur in der Theorie vorkommen kann.

Der Schutz des Konstruktors wird dann wichtig, wenn man z.B. eine Klasse erzeugt, die z.B. einen Thread-Pool erstellt und diesem Thread-Pool auch sich selbst als Referenz mitgibt.

Hiermal als völlig ungeschützte Klasse:

Delphi-Quellcode:

TMyThreadPool = class
private
  _Pool : TList<TPoolThread>;
public
  constructor Create( PoolSize : integer );
  // Some Properties ...
end;

constructor TMyThreadPool.Create;
begin
  inherited Create;
  _Pool := TObjectList<TPoolThread>.Create;
  while _Pool.Count < PoolSize do
    _Pool.Add( TPoolThread.Create( Self ) );
  // Init Some Properties
end;

Mit meiner Lösung bekomme ich die Klasse 100% Thread-Safe mit relativ geringem Aufwand.
Eine Ableitung von der Klasse TMyThreadPool geht auch sehr charmant und bleibt 100% Thread-Safe

[r2c2]

Man könnte hier natürlich darüber streiten, ob die Pool-Threads den Pool überhaupt kennen sollten, aber hier hast du recht. Immer, wenn man im Konstruktor einen Thread erzeugt und dem Self mitgibt hat man das Problem und deine Lösung hilft hier. OK, verstanden.

mfg

Christian

[Furtbichler]

Ich bleibe dabei: Trenne Instantiierung von Initialisierung und man hat viel mehr Zeit für andere Sachen. Weil man sich nicht mit solchem "Quark" beschäftigen muss. Wobei das Problem an sich schon recht interessant ist.

[r2c2]

Zitat:

Zitat von Furtbichler (Beitrag 1138011)

Ich bleibe dabei: Trenne Instantiierung von Initialisierung und man hat viel mehr Zeit für andere Sachen. Weil man sich nicht mit solchem "Quark" beschäftigen muss.

Kommt drauf an. Wenn die Instanziierung ausreicht, um das Objekt in nen konsistenten zustand zu versetzen, ist das OK und oft die bessere Lösung. Wenn man immer initialize() aufrufen muss, hat man ne weitere Fehlerquelle geschaffen, was ich definitiv vermeiden würde.

mfg

Christian