TCriticalSection: Einmal global oder immer lokal erstellen?

[jensw_2000]

Ich grabe den Thread noch einmal aus.

In der Regel verfolge ich den Ansatz, dass sich jede Objekt-Instanz, in der Daten manipuliert werden können, selbst absichert.

Bisher hat das immer recht gut funktioniert.

In meinem aktuellen Projekt such ich mir aber schon seit Tagen den Wolf wegen diverser spontaner Deadlocks.
Ich zweifle daran, ob ich das Locking richtig verstanden habe

Liegt das generell an meiner "Art" die Criticalsections zu bauen?

Wenn ja, gebt mir doch bitte kurz Bescheid, dann kann ich mir das durchsuchen von 100ten units ersparen und verwende globale Variablen für die CriticalSections.

Hier mal 2 vereinfachte Beispiele:

Bei der folgenden "Jobliste für WorkerThreads" sollte doch ein Thread Jobs hinzufügen und mehrere Workerthreads Jobs abholen können oder?
Und... kann ein Timer im MainForm sicher label1.caption := inttostr(MyJoblist.count)); aufrufen?

Delphi-Quellcode:

type
  TJoblist = class(TObjectList)  // in Wahrheit typensicherere Objectlist
  private
    FCS: TCriticalSection;
  public
    constructor Create; reintroduce;
    destructor Destroy; override;

    function GetNextJob(out aJob: TJob): boolean;
    procedure AddJob(aJob: TJob);
  end;

...
...

implementation

{ TJoblist }

procedure TJoblist.AddJob(aJob: TJob);
begin
  FCS.Enter;
  try
    Add(aJob);
  finally
    FCS.Leave;
  end;
end;

constructor TJoblist.Create();
begin
  inherited Create(True); // OwnsObjects
  FCS := TCriticalSection.Create;
end;

destructor TJoblist.Destroy;
begin
  FCS.Enter;
  try
    Clear;
  finally
    FCS.Leave;
  end;
  FCS.Free;
  inherited;
end;

function TJoblist.GetNextJob(out aJob: TJob): boolean;
begin
  FCS.Enter;
  try
    if Count > 0 then
    begin
      aJob := TJob(Extract(Items[0]));
      result := assigned(aJob);
    end
    else
    begin
      aJob := nil;
      result := false;
    end;
  Finally
    FCS.Leave;
  end;

  // ReleaseSemaphore usw. ...
end;

Meine WorkerThreads haben auch eine "interne" CriticalSection.
Über diese sichere ich z.B. die Setter zum aktualisieren von Thread-Properties ab.

Ist die "exemplarische" <Status> Property Thread-Safe?
Also.. könnte mein "Timer auf dem Mainform" beispielsweise sicher durch den Workerthread-Pool laufen und alle Workerthread-Staties in eine Listbox schreiben?

Delphi-Quellcode:

Listbox1.clear;
for i:= 0 to Threadpool.Count -1 do
begin
  Listbox1.items.add('Thread-'+inttostr(i)+' Status : '+ StatusToString( ThreadPool[i].Status) ); 
end;

Hier die vereinfachte Thread-Klasse:

Delphi-Quellcode:

  {$M+}
  TMyThread = class(TThread)
  private
    { private-Deklarationen }
    FCS: TCriticalSection;
    fStatus: TThreadStatus;
    fJoblist: TJoblist;
    procedure SetStatus(const Value: TThreadStatus);
    procedure SetJoblist(const Value: TJoblist);
  protected
    { protected-Deklarationen }
    procedure Execute; override;
  public
    { public-Deklarationen }
    constructor Create(); reintroduce;
    destructor Destroy; override;
  published
    { published-Deklarationen }
    property Status: TThreadStatus read fStatus write SetStatus;
    property JobList: TJoblist read fJoblist write SetJoblist; 
  end;

 ...

implementation

...
{ TMyThread }

constructor TMyThread.Create();
begin
  inherited Create(true);
  FCS := TCriticalSection.Create;
  FreeOnTerminate := False;  // wird von extern freigegeben;

end;

destructor TMyThread.Destroy;
begin

  FCS.Free;
  inherited;
end;

procedure TMyThread.Execute;
  var ThreadJob: TJob;
begin
  inherited;

  Status := stConfig;
  // ...
  while not Terminated do
  begin
     Status := stIdle;

     // WaitForMultipleObjects( .....)
     Status := stWorking;
     Assert(assigned(Joblist),'keine Joblist zugewiesen');

     if JobList.GetNextJob( ThreadJob ) then
     begin  
       // .. mach was mit dem Job
       // ThreadJob.Free;
     end; 
  end;

  Status := stFinalize;
  // abschließende Schritte ...

  Status := stTerminated;
end;

procedure TMyThread.SetStatus(const Value: TThreadStatus);
begin
  FCS.Enter;
  try
    fStatus := Value;
  finally
    FCS.Leave;
  end;
end;

procedure TMyThread.SetJoblist(const Value: TJoblist);
begin
  FCS.Enter;
  try
    fJoblist := Value;
  finally
    FCS.Leave;
  end;
end;

Grüße,
Jens

[Luckie]

CriticalSections müssten schon global sein, weil alle Thread sie ja kennen müssen.

[jensw_2000]

Ich hatte es aus diversen Dokus und HowTos so herausgelesen, dass die CriticalSections "so global" sein müssen, dass alle Threads die selbe CriticalSection Instanz verwenden können.

Im globalen Scope machen das auch viele, aber nicht alle...

Bei der Joblist beispielsweise ist doch sichergestellt, das alle Threads mit der einen "FCS Instanz" in der Jobliste arbeiten.

Der hier macht es genauso.
http://delphihaven.wordpress.com/code/tsimplethreadedqueue-variants/


Schweres Thema ..


PS:

Zitat:

Bei der Joblist beispielsweise ist doch sichergestellt, das alle Threads mit der einen "FCS Instanz" in der Jobliste arbeiten.

... vorausgesetzt, man greift nur über die "abgesicherten" Methoden auf die Joblist zu.
Ein "externes" MyJoblist.add(...); darf narürlich nicht sein.


PS v2:
Die Ursache meiner Dead-Locks habe ich Dank Sebastian Jänicke völlig unerwartet gefunden

http://www.delphipraxis.net/1137912-post3.html

Nun ist nur nich die Frage übrig, ob ist die CriticalSections richtig benutze?

[Sir Rufo]

Zitat von Luckie:

CriticalSections müssten schon global sein, weil alle Thread sie ja kennen müssen.

Nein, müssen schon mal gar nicht. Und was interessiert es den Thread A wenn in Thread B Daten geändert werden. Möglich ja, aber warum sollte man sich eine Blockade für alle Threads bauen?
Ich dachte immer, die sollen so schnell als möglich ihren Job machen und da ist das Ausbremsen doch völlig kontraproduktiv.

Zitat von Oberlehrer:

Eine CriticalSection muss in dem Kontext vorhanden sein, in dem auch die Daten liegen, die durch die CriticalSection geschützt werden sollen.

Bei einer globalen Variable muss man eine globale CriticalSection haben, aber wer benutzt schon globale Variablen.

Um den Datenzugriff zu schützen reicht es nicht nur den Schreibzugriff abzusichern, sondern auch den Lesezugriff!
Denn was soll denn zurückgeliefert werden, wenn während des Lesens von einem anderen Thread der Wert geändert wird? Die Hälfte vom alten und die Hälfte vom neuen Wert?

Hier mal das Beispiel für eine Klasse mit einer geschützten Eigenschaft.
(Das Erzeugen und Zerstören der CS lasse ich mal weg)

FALSCH:

Delphi-Quellcode:

TMyClass = class
strict private
  _CS : TCriticalSection;
private
  fMyValue : string;
  procedure SetMyValue( const Value : string );
public
  property MyValue : string read fMyValue write SetMyValue;
end;

procedure TMyClass.SetMyValue( const Value : string );
begin
  _CS.Enter;
  try
    fMyValue := Value; // geschützter Schreib-Zugriff
  finally
    _CS.Leave;
  end;
end;

RICHTIG:

Delphi-Quellcode:

TMyClass = class
strict private
  _CS : TCriticalSection;
private
  fMyValue : string;
  function GetMyValue : string;
  procedure SetMyValue( const Value : string );
public
  property MyValue : string read GetMyValue write SetMyValue;
end;

function TMyClass.GetMyValue : string;
begin
  _CS.Enter;
  try
    Result := fMyValue;  // geschützter Lese-Zugriff
  finally
    _CS.Leave;
  end;
end;

// Schreibzugriff geschützt

procedure TMyClass.SetMyValue( const Value : string );
begin
  _CS.Enter;
  try
    fMyValue := Value; // geschützter Schreib-Zugriff
  finally
    _CS.Leave;
  end;
end;

[jensw_2000]

Zitat:

Um den Datenzugriff zu schützen reicht es nicht nur den Schreibzugriff abzusichern, sondern auch den Lesezugriff!
Denn was soll denn zurückgeliefert werden, wenn während des Lesens von einem anderen Thread der Wert geändert wird? Die Hälfte vom alten und die Hälfte vom neuen Wert?

Das könnte 1-2 "Effekte" erklären.

Verstehe ich das richtig, dass in dem Beispiel "nur" der Codeabschnitt im Getter und Setter durch die CS gelockt wird und nicht das Feld "fMyValue"?

Wenn dein Beispiel "TMyClass" von TThread abgeleitet wäre, müsste man demnach auch klassenintern (besonders in TThread.Execute) darauf achten, dass man immer über die Property auf fMyValue zugreift und nie direkt. Richtig?

[Sir Rufo]

Zitat von jensw_2000:

Wenn dein Beispiel "TMyClass" von TThread abgeleitet wäre, müsste man demnach auch klassenintern (besonders in TThread.Execute) darauf achten, dass man immer über die Property auf fMyValue zugreift und nie direkt. Richtig?

Jeder Zugriff auf Daten, die auch von ausserhalb (hier das Feld fMyValue der Thread-Instanz über eine public property) erreicht werden können, sollte nur geschützt erfolgen.

Das muss aber nicht ausschließlich über die Property erfolgen.

Delphi-Quellcode:

procedure TMyThread.Execute;
begin

  MyValue := 'geschützt';

  fMyValue := 'NICHT geschützt - es droht eine AV';

  CS.Enter;
  try
    fMyValue := '';
    for i := 1 to 5 do
      fMyValue := fMyValue + 'geschützt';
  finally
    CS.Leave;
  end;

end;

[schlecki]

Ich benutze auch immer gern den TMultiReadExclusiveWriteSynchronizer - langer Name

Der hat den Vorteil, dass sich gleichzeitig Lesen lässt, aber das Schreiben exklusiv erfolgen muß - naja, der Name verräts eigentlich auch ^^
Wichtig ist nur, dass man _immer_!!! entweder den Lese- oder den Schreibzugriff aufrufen darf. Ansonsten kann es zu deadlocks kommen.

[Luckie]

Zitat von Sir Rufo:

Zitat von Luckie:

CriticalSections müssten schon global sein, weil alle Thread sie ja kennen müssen.

Nein, müssen schon mal gar nicht. Und was interessiert es den Thread A wenn in Thread B Daten geändert werden.

Ich bin natürlich davon ausgegangen, dass die Threads auf die selben Daten zugreifen. Aber ich dachte das wäre klar.