TThreadStringList

[Klaus01]

Guten Abend,

da ich einem Projekt mit Threads arbeite und dazu auch aus den Threads
auf eine StringList zugegriffen wird, möchte ich diese gerne Threadsafe machen.
Leider kann ich die Klasse nicht vollständig bei mir testen - ich habe nur eine Single core
System - da hatte auch der 'nicht Thread safe" Zugriff keine Probleme bereitet.
Das Zielsystem ist aber eine Mehrprozessormaschine.
Auch die ThreadStringList läuft auf meinem single core System.

Daher meine Frage, kann mal wer über die Klasse schauen und Kritik üben.
Bin mir nicht sicher ob nicht ein paar grobe Schnitzer drin sind.

Delphi-Quellcode:

unit UThreadStringList;

interface
uses
  classes,syncObjs,sysUtils;

type

  TThreadStringList = class
    private
      FStringList : TStringList;
      FCriticalSection : TCriticalSection;
    public
      constructor create;
      destructor destroy;
      function lockStringList:TStringList;
      procedure unlock;
      procedure add(AItem: AnsiString);
      function get(AIdx: Integer): AnsiString;
      procedure saveToFile(AFilePath: AnsiString);
      procedure remove(AIdx: Integer);
      function count:Integer;
    end;

implementation

constructor TThreadStringList.create;
begin
  FStringList := TStringList.Create;
  FCriticalSection := TCriticalSection.create;
end;

destructor TThreadStringList.destroy;
begin
  freeAndNil(FStringList);
  freeAndNil(FCriticalSection);
end;

function TThreadStringList.lockStringList:TStringList;
begin
  FCriticalSection.Acquire;
  result := FStringList;
end;

procedure TThreadStringList.unlock;
begin
  FCriticalSection.Release;
end;

procedure TThreadStringList.add(AItem: AnsiString);
var
  sl : TStringList;
begin
  sl := lockStringList;
  sl.Add(AItem);
  unlock;
end;

function TThreadStringList.get(AIdx: Integer): AnsiString;
var
  sl : TStringList;
begin
  sl := lockStringList;
  result := sl[AIdx];
  unlock;
end;

procedure TThreadStringList.saveToFile(AFilePath: string);
var
  sl : TStringList;
begin
  sl := lockStringList;
  try
    sl.SaveToFile(AFilePath);
  finally
    unlock;
  end;
end;

procedure TThreadStringList.remove(AIdx: Integer);
var
  sl : TStringList;
begin
  sl := lockStringList;
  sl.Delete(AIdx);
  unlock;
end;

function TThreadStringList.count:Integer;
var
  sl : TStringList;
begin
  sl := lockStringList;
  result := sl.count;
  unlock;
end;

end.

Danke für das Lesen.

Grüße
Klaus

[himitsu]

Hab's nicht getestet,
aber ja, es sollte ist Threadsave sein.

nur bei den Zugriffen würde ich überall noch etwas mehr absichern:

Delphi-Quellcode:

var
  sl : TStringList;
begin
  sl := lockStringList;
  try
    // mache irgendwas
    // z.B.: sl.Delete(AIdx);
  finally
    unlock;
  end;
end;

[edit]
ich glaub so klingt es besser.

[Klaus01]

Hallo himitsu,

ist eine gute Idee, werde ich noch machen.

Grüße
Klaus

[himitsu]

Eine andere, zwar aufwändigere, aber dafür flexiblere Variante wäre
- direkt von TStringList abzuleiten,
- das FCriticalSection : TCriticalSection; hinzuzufügen
- und dann alles, welches in TStringList und TStrings als Virtual gekennzeichnet ist zu überschreiben

Hat noch den Vorteil, daß es wie eine normale StringListe verwendet werden kann.

Delphi-Quellcode:

procedure TThreadStringList.Lock;
begin
  FCriticalSection.Acquire;
end;

procedure TThreadStringList.Unlock;
begin
  FCriticalSection.Release;
end;

// und dann überall

procedure TThreadStringList.SaveToFile(const FileName: string);
begin
  Lock;
  try
    inherited SaveToFile(FileName);
  finally
    Unlock;
  end;
end;

function TThreadStringList.IndexOf(const S: string): Integer;
begin
  Lock;
  try
    Result := inherited IndexOf(S);
  finally
    Unlock;
  end;
end;

PS: The Generics Stack
also nur zur Info ... in deinem "älteren" Delphi läuft es ja nicht

[Astat]

Zitat:

Zitat von Klaus01

..kann mal wer über die Klasse schauen und Kritik üben.

Hallo Klaus01, bei Deiner Klasse solltest Du lockStringList und unlock als privat deklarieren.
Denn wenn Public, und es wird lockStringList aufgerufen, und mit dieser dann gearbeitet, ist das Teil nicht THreadsave.

Wenn nun lockStringList privat ist, so kannst Du es gleich weglassen.
Möchtest Du wirklich mit der Stringliste ausserhalb der Classe arbeiten, musst Du auch einen Schutzblock nach aussen geben.

Delphi-Quellcode:

  TThreadStringList = class
    private
      FStringList : TStringList;
      FCriticalSection : TCriticalSection;
      function lockStringList: TStringList;
      procedure unlock;
    public
      constructor create;
      destructor destroy;

      procedure LockList;
      procedure UnlockList;


      procedure add(AItem: AnsiString);
      function get(AIdx: Integer): AnsiString;
      procedure saveToFile(AFilePath: AnsiString);
      procedure remove(AIdx: Integer);
      function count:Integer;
    end;


procedure TForm1.Button1Click(Sender: TObject);
begin
  ThreadStringList.LockList;
  try
    //-- hier Stringliste aus Classe holen und arbeiten
  finally
    ThreadStringList.UnlockList;
  end;
end;

lg. Astat

[Klaus01]

Hallo Astat,

gedacht hatte ich mir das eigentlich so:

Delphi-Quellcode:

procedure TForm1.Button1Click(Sender: TObject);
var 
  sl : TStringList;
begin
  sl  := ThreadStringList.lockStringList;
  try
    //-- hier etwas mit der Stringlist sl anstellen
  finally
    ThreadStringList.Unlock;
  end;
end;

[edit]
Ich habe mich da ein wenig an TThreadList angelehnt.
Dort wird auch mit ThreadList.lockList die TList übergeben.
[/edit]
Laut Deiner Aussage wäre dann sl ohne Schutz, obwohl ich
eine CriticalSection gesetzt habe?
Ist diese CriticalSection nur innerhalb der Instanz gültig?

Grüße
Klaus

[Astat]

Hallo Klaus,

EDIT ist nicht richtig, liste ist doch geschützt. Bei Thread #8 Himitsu gehts korrekt weiter
Ja die Liste ist ohne Schutz. Die Liste darf ausserhalb der Classe nicht von mehreren Threads bearbeitet werden.
Der Zugriff muss über die Klassenmethoden erfolgen, darum locklist in den privaten Bereich verschieben, und dessen
Result (Liste) nur innerhalb der Classe verwenden.

Ich würde das in etwa so verwenden.

Delphi-Quellcode:

type
  TThreadStringList = class
    private
      FStringList : TStringList;
      FCriticalSection : TCriticalSection;
    public
      constructor create;
      destructor destroy;
      procedure Add(AItem: AnsiString);
      function Get(AIdx: Integer): AnsiString;
      procedure SaveToFile(AFilePath: AnsiString);
      procedure Remove(AIdx: Integer);
      function Count: Integer;
    end;

implementation

constructor TThreadStringList.create;
begin
  FCriticalSection := TCriticalSection.create;
  FStringList := TStringList.Create;
end;

destructor TThreadStringList.destroy;
begin
  freeAndNil(FStringList);
  freeAndNil(FCriticalSection);
end;

procedure TThreadStringList.add(AItem: AnsiString);
begin
  FCriticalSection.Acquire;
  try
    FStringList.Add(AItem);
  finally
    FCriticalSection.Release;
  end;
end;

function TThreadStringList.get(AIdx: Integer): AnsiString;
begin
  FCriticalSection.Acquire;
  try
    result := FStringList[AIdx];
  finally
    FCriticalSection.Release;
  end;
end;

procedure TThreadStringList.saveToFile(AFilePath: string);
begin
  FCriticalSection.Acquire;
  try
    FStringList.SaveToFile(AFilePath);
  finally
    FCriticalSection.Release;
  end;
end;

procedure TThreadStringList.remove(AIdx: Integer);
begin
  FCriticalSection.Acquire;
  try
    FStringList.Delete(AIdx);
  finally
    FCriticalSection.Release;
  end;
end;

function TThreadStringList.count:Integer;
begin
  FCriticalSection.Acquire;
  try
    result := FStringList.count;
  finally
    FCriticalSection.Release;
  end;
end;

lg. Astat

[himitsu]

Zitat:

Denn wenn Public, und es wird lockStringList aufgerufen, und mit dieser dann gearbeitet, ist das Teil nicht THreadsave.

Doch, sie ist dennoch sicher, da dann die CS sperrt und somit andere keinen Zugriff haben.

Vorteil bei der offenen deklaration, man kommt auch mal richtig an die StringListe ran, falls man sie z.B. mal sortieren oder sonstwas damit machen will.

Delphi-Quellcode:

sl := ThreadStringList.lockStringList;
try
  // mache irgendwas
  // z.B.: sl.Delete(AIdx);
finally
  ThreadStringList.unlock;
end;

Ob man sowas nun außerhalb macht oder innerhalb der ThreadStringList, ist dabei vom Schutz her egal.

Bin da zwar zum Großteil mehr der Theoretiker, aber glaubs mir einfach ... mit den angemerkten Änderungen isses OK.
Und nein, ich hab den Code jetzt garnantiert nicht durchgesehn, aber ich seh diesbezüglich auch keine Schwachstellen ... solange man nicht an der CS vorbei auf die StringList zugreifen kann, was ja nicht der Fall ist.

Gut, man könnte jetzt zwar denken, daß String dankt seiner Referenzzählung jetzt noch ein Problem darstellen kann, da er nach dem Get ja nur eine Referenz auf den String in der internen StringList besitzt, aber die Referenzzähung bei Strings ist threadsicher ausgelegt (auch wenn man das knuffige "LOCK" in den entsprechenden Assemblercodes leicht übersieht) und wenn dann noch der Speichermanager auf Threadbetrieb eingestellt ist, dann gibt es diesbezüglich keine Probleme.


Der Einzige Schwachpunkt läge darin, wenn man dann absichtlich den Schutz umgeht, aber wer das macht, der muß dann auch mit den Konsequenzen leben. :warn:

Delphi-Quellcode:

sl := ThreadStringList.lockStringList;
try
  // mache irgendwas
  // z.B.: sl.Delete(AIdx);
finally
  ThreadStringList.unlock;
end;
// mache was unsicheres mit der StringList
SL.Add('fdsafds');

[Astat]

Zitat:

Zitat von himitsu

Ob man sowas nun außerhalb macht oder innerhalb der ThreadStringList, ist dabei vom Schutz her egal.

Ja, :thumb: komisch, :gruebel: wenn man genau hinschaut is es aber so, würde das mal Betriebsblind nennen! :wall:

Danke für Info

[Klaus01]

.. ich danke Euch beiden auch für die Infos und die rege Diskussion.

In den Klassenmethoden habe ich jetzt try.. finally benutzt.
Den Rest lasse ich ersteinmal so.
Wenn ich die TThreadStringList mit Ableitung von TStringlist fertig habe,
werde ich sie hier noch anhängen.

Danke nochmals.

Grüße
Klaus