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:

zusammenfalten · markieren

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?

zusammenfalten · markieren

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:

zusammenfalten · markieren

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.

zusammenfalten · markieren

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:

zusammenfalten · markieren

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.

Zitat:

markieren

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.

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

markieren

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.

zusammenfalten · markieren

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.

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

mfg

Christian

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.

Zitat von r2c2:

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?

zusammenfalten · markieren

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.

zusammenfalten · markieren

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.

markieren

Delphi-Quellcode:

CS.Enter;
CS.Free;

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

Zitat von Sir Rufo:

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?

zusammenfalten · markieren

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

Zitat von r2c2:

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 von r2c2:

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.

Zitat von Sir Rufo:

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