TThread Syncronise & TCriticalSection.Enter

[Jonas Shinaniganz]

Hallo Forum, aus Interesse habe ich in aller Schnelle folgende App erstellt:

Es wird in einem IP-Bereich für jede Adresse eine NSLOOKUP Anfrage gestellt.

Delphi-Quellcode:

unit Unit1;

interface

uses
  System.SysUtils, System.Types, System.Variants, System.UITypes,
  System.Classes, FMX.Types, FMX.Graphics, FMX.Dialogs, FMX.Types3D, FMX.Forms,
  FMX.Forms3D, FMX.Controls3D, IdGlobal, IdStack, FMX.Layers3D, FMX.StdCtrls,
  FMX.Controls, FMX.Layouts, FMX.Memo, SyncObjs;

type
  TForm1 = class(TForm3D)

    Memo: TMemo;
    ProgressBar: TProgressBar;
    Layer3D1: TLayer3D;
    AniIndicator1: TAniIndicator;
    procedure Form3DCreate(Sender: TObject);
  private
    Fprogress: Integer;
    procedure Setprogress(const Value: Integer);
  public
    Lock : TCriticalSection;
    property progress : Integer read Fprogress write Setprogress;
  end;

  MyThread = class(TThread)

  protected
    Ip : Integer;
    procedure Execute; override;
    procedure UnserThread;
  public
    constructor Create(next : Integer);

  end;

var
  Form1: TForm1;
  thrd : MyThread;
  stack : TIdStack;

const
  NET : String = '172.17.4.';

implementation

{$R *.fmx}

procedure MyThread.UnserThread;
var
  str : String;
begin
  TIdStack.IncUsage;
  try
    str := Gstack.HostByAddress(NET + IntToStr(Ip));
  except on E: Exception do
    str := 'inexistent domain';
  end;
  TIdStack.DecUsage;

  Synchronize(
  procedure
  var
    I : Integer;
  begin
    I := 0;
    Form1.Lock.Enter;

      if Form1.Memo.Lines.Count <= 0 then
        Form1.Memo.Lines.Insert(0, '...' + Format('%3d',[Ip])+ ' - ' + str)
      else
      begin
        while ((Form1.Memo.Lines[I]) < ('...' + Format('%3d',[Ip])+ ' - ' + str)) or (I <= Form1.Memo.Lines.Count) do
          inc(I);
        Form1.Memo.Lines.Insert(I, '...' + Format('%3d',[Ip])+ ' - ' + str);
      end;

    Form1.Lock.Leave;

    Form1.ProgressBar.Value := Form1.ProgressBar.Value + 1;
    Form1.progress := Form1.Progress + 1;
  end);

end;

procedure TForm1.Form3DCreate(Sender: TObject);
var
  i : integer;
begin
  Lock := TCriticalSection.Create;

  for I := 1 to 254 do
    thrd := MyThread.Create(I);

  ProgressBar.Min := 1;
  ProgressBar.Max := 254;
  ProgressBar.Value := 1;
  progress := 1;

  //Memo.Lines.Add('Parse Net: ' + NET + '...');
end;

{ MyThread }

constructor MyThread.Create(next : Integer);
begin
  inherited Create;
  Ip := next;
end;

procedure MyThread.Execute;
begin
  inherited;
  UnserThread;
end;

procedure TForm1.Setprogress(const Value: Integer);
begin
  Fprogress := Value;
  if Fprogress = 254 then
    AniIndicator1.Enabled := False;
end;

end.

Sobald der erste Thread in der Zeile

Delphi-Quellcode:

Form1.Lock.Enter;

ankommt, wird ja für alle Anderen Threads der Zugriff/Weiterverarbeitung blockiert die an dieser Stelle ankommen, bis der schnellste/erste Thread mit der Ausführung bei

Delphi-Quellcode:

Form1.Lock.Leave;

ankommt. Synchronize brauche ich zusätzlich, um Änderungen an der Oberfläche/Hauptthread machen zu können.

Ich habe wohl einen Denkfehler/Missverständnis. Es ist das Erste mal, dass ich mit diesen beiden Klassen Arbeite. :oops:
Beim Debuggen jedenfalls läuft es garnicht wie ich mir das vorstelle. Er springt wild hin und her, was ja bei 200+ Threads nicht ganz unverständlich ist.
Ich denke trotzdem das jemand mit Erfahrung bestimmt einen guten Tipp hat. Zur Abwechslung mache ich es mir also mal leicht und frage gleich!

Grüße!

[himitsu]

Wenn dieses

Delphi-Quellcode:

Form1.Lock.Enter;

immer nur in dem Syncronize steht, bzw. immer nur im Hauptthread, dann kann da sowieso immer nur ein Thread vorbeikommen, da immer nur ein Thread gleichzeitig im Hauptthread arbeiten kann.

Die anderen Threads warten also schon beim Syncronize, bis der Thread zurückkommt und auch die VCL macht nichts, während ein Thread synchronisiert ist.


CriticalSections bringen nur etwas, wenn sie in verschiedenen Threads verwendet werden, um gemeinsame Resourcen gegenseitig abzusichern.
Genauso gut kann man alles in einen Thread synchronisieren (hier der Hauptthread) und ist damit auch abgesichert, da ja letztendlich nur der eine Thread darauf zugreift.



PS: Alles in dem Syncronize gehört doch zur Form? (wie man schön erkennen kann, da überall das Form1 davorgeschrieben werden mußte)
Also gehört das auch in eine Methode der Form und von dem/den Thread(s) auch ruft man dann diese Methode auf.
Fazit: Sobald du sehr oft auf solche globale Variablen zugreifen mußt, dann stimmt meistens irgendwas im Design nicht.

[Sir Rufo]

Nur so nebenbei:

Die ProgressBar sollte von 0..254 anzeigen und bei 0 starten, denn wenn 254 mal 1 um 1 erhöht wird, dann hat man 255 :)

[Blup]

Da war auch noch ein Fehler in der While-Schleife.
Mein Vorschlag, ohne das ich jetzt getestet hab:

Delphi-Quellcode:

unit Unit1;

interface

uses
  System.SysUtils, System.Types, System.Variants, System.UITypes,
  System.Classes, FMX.Types, FMX.Graphics, FMX.Dialogs, FMX.Types3D, FMX.Forms,
  FMX.Forms3D, FMX.Controls3D, IdGlobal, IdStack, FMX.Layers3D, FMX.StdCtrls,
  FMX.Controls, FMX.Layouts, FMX.Memo, SyncObjs;

type
  TForm1 = class(TForm3D)
    Memo: TMemo;
    ProgressBar: TProgressBar;
    Layer3D1: TLayer3D;
    AniIndicator1: TAniIndicator;
    procedure Form3DCreate(Sender: TObject);
  private
    FProgress: Integer;
    procedure SetProgress(AValue: Integer);
    procedure DoOnThreadTerminate(Sender: TObject);
  public
    property Progress: Integer read FProgress write SetProgress;
  end;

  TMyThread = class(TThread)
  protected
    FIp: Integer;
    FStr: string;
    procedure Execute; override;
  public
    constructor Create(AIP: Integer; AFinishEvent: TNotifyEvent);
    property Ip: Integer read FIp;
    property Str: String read FStr;
  end;

var
  Form1: TForm1;
  stack : TIdStack;

const
  NET : String = '172.17.4.';

implementation

{$R *.fmx}

procedure TForm1.Form3DCreate(Sender: TObject);
var
  i : integer;
begin
  ProgressBar.Min := 0;
  ProgressBar.Max := 254;
  Progress := 0;

  for i := 1 to 254 do
    MyThread.Create(I, DoOnThreadTerminate);
end;

procedure TForm1.DoOnThreadTerminate(Sender: TObject);
var
  sIpStr: string;
  i: Integer;
begin
  with TMyThread(Sender) do
    sIpStr := Format('...%3d - %s', [Ip, Str]);

  i := 0;
  while (i < Memo.Lines.Count) and (Memo.Lines[i] < sIpStr) do
    Inc(i);

  Memo.Lines.Insert(i, sIpStr);

  Progress := Progress + 1;
end;

procedure TForm1.SetProgress(AValue: Integer);
begin
  FProgress := Value;
  ProgressBar.Value := AValue
  if FProgress = 254 then
    AniIndicator1.Enabled := False;
end;

{ MyThread }

constructor TMyThread.Create(AIp: Integer; AFinishEvent: TNotifyEvent);
begin
  inherited Create;
  FIp         := AIp;
  OnTerminate := AFinishEvent;
end;

procedure TMyThread.Execute;
begin
  inherited;

  TIdStack.IncUsage;
  try
    FStr := Gstack.HostByAddress(NET + IntToStr(FIp));
  except on E: Exception do
    FStr := 'inexistent domain';
  end;
  TIdStack.DecUsage;
end;

end.

[Jonas Shinaniganz]

Delphi-Quellcode:

unit frmMain;

interface

uses
  System.SysUtils, System.Classes, FMX.Forms3D, IdGlobal, IdStack,
  FMX.Layers3D, FMX.StdCtrls,  FMX.Layouts, FMX.Memo, FMX.Edit, FMX.Controls,
  FMX.Controls3D, FMX.Types;

type
  TMainForm = class(TForm3D)
    Memo: TMemo;
    ProgressBar: TProgressBar;
    Layer3D1: TLayer3D;
    AniIndicator1: TAniIndicator;
    NumberBox1: TNumberBox;
    Label1: TLabel;
    NumberBox2: TNumberBox;
    NumberBox3: TNumberBox;
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
  private
    Output : TStringlist;
    FProgress : Integer;
    procedure AddToMemo(Ip : Integer; Str : String);
    procedure Setprogress(const Value: Integer);
  public
    property Progress : Integer read FProgress write SetProgress;
  end;

  TLookupThread = class(TThread)
  protected
    Ip : Integer;
    procedure Execute; override;
  public
    constructor Create(Next : Integer);
  end;

var
  MainForm : TMainForm;
  NET : String = '172.17.4.';

implementation

{$R *.fmx}

procedure TMainForm.AddToMemo(Ip : Integer; Str : String);
begin
  Output.Add('...' + Format('%3d',[Ip])+ ' - ' + str);
  Progress := Progress + 1;
end;

procedure TMainForm.Button1Click(Sender: TObject);
var
  i : integer;
  thrd : TLookupThread;
begin
  Output := TStringList.Create;

  NET := NumberBox1.Value.ToString + '.' + NumberBox2.Value.ToString + '.' + NumberBox3.Value.ToString + '.';

  for I := 1 to 254 do
    thrd := TLookupThread.Create(I);

  ProgressBar.Min := 0;
  ProgressBar.Max := 253;
  Progress := 0;

  AniIndicator1.Enabled := true;
  AniIndicator1.Visible := true;
end;

constructor TLookupThread.Create(Next : Integer);
begin
  inherited Create;
  Ip := Next;
end;

procedure TLookupThread.Execute;
var
  str : String;
begin
  inherited;
  TIdStack.IncUsage;
  try
    str := Gstack.HostByAddress(NET + IntToStr(Ip));
  except on E: Exception do
    str := 'inexistent domain';
  end;
  TIdStack.DecUsage;

  Synchronize(
  procedure
  begin
    MainForm.AddToMemo(Ip, Str);
  end);

  Self.Free;
end;

procedure TMainForm.Setprogress(const Value : Integer);
begin
  FProgress := Value;
  ProgressBar.Value := FProgress;
  if FProgress = 254 then
  begin
    AniIndicator1.Enabled := False;
    AniIndicator1.Visible := False;
    Output.Sort;
    Output.Insert(0, 'parse net: ' + NET + '...');
    Memo.Lines.Assign(Output);
    Output.Free;
  end;
end;

end.

Naja ich habe es so angepasst. Programm gibt's im Anhang, leicht erweitert.
Die While-Schleife macht ja einen Insertionsort... Das war auch vorher der Haken, habe es dann so ähnlich korrigiert. Danach fand ich das häufige hinzufügen von Lines zum Memo aber nicht mehr schön. Nutze letztlich eine TStringlist. Für mich ist das jetzt auch schon wieder abgehakt :) Danke an euch.

Edit:
Warum gibt es eigentlich keine Warnung das

Zitat:

stack : TIdStack;

nie benutzt wird? @Blup Das mit dem Event ist natürlich schöner designed.

[Blup]

Für globale Variablen kann es so eine Warnung nicht geben.
Der Compiler weis nicht ob diese Unit eventuell auch in anderen Projekten verwendet wird, die genau diese Variable benötigen.

[himitsu]

Globale Variablen sind halt böse. :stupid: