Wann und wie Threadvariable freigeben

nru · Hält's aus hier Beiträge: 9 Erhaltene Danke: 1

Hallo Ihr,

ich glaube, ich bräuchte bitte mal etwas CodeReview von Euch.

Ausgangslage:
In einer DB soll je Datensatz ein und derselbe Thread erneut gestartet werden (Zweck: Download, Images).

Stand der Dinge:
Im dem im Thread erstellenden Event kann ich natürlich feststellen, ob ein solcher Thread bereits existiert (der vom letzten Datensatz) und den dann terminieren und nach WaitFor auch freigeben (FreeOnTerminate ist nicht gesetzt).

ABER ...

Problem:
Wann genau free'e ich den letzten Thread? Den, der u.U. normal zum Abschluss gekommen ist, weil kein weiterer RecordChange anstand oder, wie in meinem Testfall hier, kein weiterer Click in der Listbox erfolgt ist?
Erst im OnClose des Forms ist mir irgendwie zu ungenau. Oder sollte ein := nil im OnTerminate-Eveent AfterThreadFinished reichen? Eigentlich nicht, da FreeOnTerminate := False ist.

Kurz gesagt, es geht mir um den abschließenden Free/FreeAndNil(), der hier irgendwie noch fehlt.

Danke für Eure Aufmerksamkeit!

Gruss
nru

Hier mal mein Testproggi (mit einer Listbox und ihrem OnClick anstelle der DB):

markieren

Delphi-Quelltext

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
69:
70:
71:
72:
73:
74:
75:
76:
77:
78:
79:
80:
81:
82:
83:
84:

type
  TForm1 = class(TForm)
    ListBox1: TListBox;
    Label1: TLabel;
    Button1: TButton;
    Label2: TLabel;
    procedure FormCreate(Sender: TObject);
    procedure ListBox1Click(Sender: TObject);
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
  private
    procedure OnThreadWork( Sender: TThread; count: Integer );
    procedure OnThreadWorkBegin( Sender: TThread; count: Integer );
    procedure AfterThreadFinished( Sender: TObject );
  public
  end;

var
  Form1: TForm1;

implementation

uses SelfThread;

{$R *.dfm}

procedure TForm1.FormCreate(Sender: TObject);
var
  i: Integer;
begin
  // DummyListBox wird nur als DB-Emulation gefüllt
  for i := 0 to 1000 do
    Listbox1.Items.Add( IntToStr( i ) );
end;

procedure TForm1.ListBox1Click(Sender: TObject);
begin

// Wechsel in TListbox

// Alten Thread stoppen, wenn vorhanden
  if Assigned( FSelfThread ) then begin
    with FSelfThread do begin
      Terminate;        {Thread beenden}
      WaitFor;          {Und reagieren lassen}
    end;
    FreeAndNil( FSelfThread );
  end;

// Neuen Thread starten
  if FSelfThread = nil then begin
    FSelfThread := TGoogleWSThread.Create(AfterThreadFinished); //Thread inaktiv erzeugen
    FSelfThread.OnWork := OnThreadWork;
    FSelfThread.Resume;                          //Thread aktivieren
  end;

end;

procedure TForm1.AfterThreadFinished( Sender: TObject );
begin
  label2.Caption := IntToStr( FSelfThread.FTime );
  // FSelfThread := nil; ???
end;
procedure TForm1.OnThreadWork( Sender: TThread; count: Integer );
begin
  label1.Caption := IntToStr( count );
end;
procedure TForm1.OnThreadWorkBegin( Sender: TThread; count: Integer );
begin
  label2.Caption := IntToStr( count );
end;

procedure TForm1.FormClose(Sender: TObject; var Action: TCloseAction);
begin
// Thread prüfen und ggf. terminieren und freigeben - Vielleicht läuft er ja noch
  if Assigned( FSelfThread ) then begin
    with FSelfThread do begin
      Terminate;        {Thread beenden}
      WaitFor;          {Und reagieren lassen}
    end;
    FreeAndNil( FSelfThread );
  end;
end;

end.

markieren

Thread Unit

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:

  TOnWorkEvent = procedure( sender: TThread; count: Integer ) of object;

  TGoogleWSThread = class(TThread)
  private
    FCounter: Integer;
    FOnWork: TOnWorkEvent;
    FOnWorkBegin: TOnWorkEvent;
    procedure DoNotifyWork;
    procedure DoNotifyWorkBegin();
  protected
    procedure Execute; override;
  public
    FTime: Integer;
    constructor Create(Callback: TNotifyEvent);
    property OnWork: TOnWorkEvent read FOnWork write FOnWork;
    property OnWorkBegin: TOnWorkEvent read FOnWorkBegin write FOnWorkBegin;
  end;

  var
    FSelfThread: TGoogleWSThread;


implementation



constructor TGoogleWSThread.Create(Callback: TNotifyEvent);
begin
  inherited Create( true );     {Thread wartet auf resume}
  // FreeOnTerminate := True;
  OnTerminate := callback;      {Wird ausgeführt, wenn Thread terminiert wurde. Normal oder per Abbruch}
end;


procedure TGoogleWSThread.Execute;
var
  i: Integer;
begin
  FCounter := 0;
  FTime := GetTickCount();
  Synchronize( DoNotifyWorkBegin );
  for i := 0 to 5000 do begin
    Inc( FCounter );
    Sleep(1);
    Synchronize( DoNotifyWork );
    if Terminated then break;     // Terminate aus Hauptthread berücksichtigen
  end;
  FTime := (GetTickCount - FTime);
end;


//
// Interne ThreadEventFunktionen, die Events des Hauptthreads auslösen, falls assigned
//
procedure TGoogleWSThread.DoNotifyWork();
begin
  if Assigned( OnWork ) then OnWork( self, FCounter );
end;
procedure TGoogleWSThread.DoNotifyWorkBegin();
begin
  if Assigned( OnWorkBegin ) then OnWorkBegin( self, FCounter );
end;

jaenicke · Verfasst: Fr 15.10.10 06:20

Da du immer wieder den Thread brauchst, bietet es sich an den einfach immer laufen zu lassen. Dann müsstest du nur noch dem Thread immer mitteilen, wenn es etwas Neues zu tun gibt und der arbeitet es dann ab.

Dafür kannst du im Thread z.B. WaitForMultipleObjects benutzen um zu warten bis die nächste Aufgabe kommt. Für Details habe ich jetzt keine Zeit, da ich gleich zur Arbeit losgehe, da kann ich aber heute Abend mehr dazu schreiben.

nru · Hält's aus hier Beiträge: 9 Erhaltene Danke: 1

jaaaa, das der Thread ständig neu erstellt wurde, war mir auch nicht soooo recht. Und da kam Dein Tip mit WaitForMultipleObjects gerade richtig. Das war der erhofften Denkanstoß, denn der hat mich dann zu TEvent gebracht. Damit hab ich jetzt auch eine Umsetung hinbekommen, die grundsätzlich auch eigentlich gut läuft. Der Thread wird nur noch einmal erstellt und per SetEvent() nur neu "gestartet", wenn der Record (bzw. Listboxitem im aktuellen Testfall) sich ändert.

Kann das so bleiben? Was meint Ihr?

Gruss
nru

Thread-Unit

markieren

Delphi-Quelltext

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
69:
70:
71:
72:
73:
74:
75:
76:
77:
78:
79:
80:
81:
82:
83:
84:
85:
86:
87:
88:
89:
90:
91:
92:
93:
94:
95:
96:
97:
98:
99:
100:
101:
102:
103:
104:
105:
106:
107:
108:
109:
110:
111:
112:
113:
114:
115:
116:
117:

type

  TOnWorkEvent = procedure( sender: TThread; count: Integer ) of object;

  TGoogleWSThread = class(TThread)
  private
    FSkip: Boolean;
    FEvent: TEvent;
    FCounter: Integer;
    FOnWork: TOnWorkEvent;
    FOnWorkBegin: TOnWorkEvent;
    FOnFinishDL: TNotifyEvent;
    procedure DoNotifyWork;
    procedure DoNotifyWorkBegin();
    procedure DoNotifyFinishDL();
  protected
    procedure Execute; override;
  public
    FTime: Integer;
    constructor Create(Callback: TNotifyEvent);
    destructor Destroy(); override;
    procedure DoDownload();
    class function IsRunning: boolean;
    property OnWork: TOnWorkEvent read FOnWork write FOnWork;
    property OnWorkBegin: TOnWorkEvent read FOnWorkBegin write FOnWorkBegin;
    property OnFinishDownload: TNotifyEvent read FOnFinishDL write FOnFinishDL;
  end;

  var
    FSelfThread: TGoogleWSThread;


implementation

var
  running:integer=0;

class function TGoogleWSThread.IsRunning:boolean;
begin
  result := (running>0);
end;
constructor TGoogleWSThread.Create(Callback: TNotifyEvent);
begin
  FSkip := False;
  FEvent := TEvent.Create(nil,true,false,'');
  inherited Create( true );     {Thread wartet auf resume}
  FreeOnTerminate := False;     {Release wird von MainThread initiiert}
  OnFinishDownload := Callback; {Eventfunktion des Hauptthreads}
end;
destructor TGoogleWSThread.Destroy();
begin
  FEvent.Free();
  inherited Destroy();
end;
procedure TGoogleWSThread.DoDownload();
begin
  if not Terminated then begin
    if IsRunning then           {Wenn Thread noch läuft, über FSkip Schleife verlassen}
      FSkip := True
    else                        {Thread läuft noch nicht oder nicht mehr, dann starten}
      FEvent.SetEvent();
  end;
end;
procedure TGoogleWSThread.Execute;
var
  wr: TWaitResult;
  i: Integer;
begin

  // Thread soll immer weiter laufen
  while not Terminated do begin

    // auf SetEvent oder Abbruch reagieren
    repeat
      wr := FEvent.WaitFor(500);
    until (wr <> wrTimeOut) or Terminated;    // Event oder Terminate feststellen

    if not Terminated then begin
      inc(running);                           // runningflag setzen
      FCounter := 0;
      FTime := GetTickCount();
      Synchronize( DoNotifyWorkBegin );
      for i := 0 to 5000 do begin
        Inc( FCounter );
        Sleep(1);
        Synchronize( DoNotifyWork );
        if Terminated or FSkip then break;    // Terminate aus Hauptthread berücksichtigen oder Skip aus DoDownload
      end;
      FTime := (GetTickCount - FTime);
      FEvent.ResetEvent();                    // Nach normalem Abschluss des Jobs Event zurücksetzen
      if not FSkip and not Terminated then    // Nur komplett abgeschlossener Download
        Synchronize( DoNotifyFinishDL );      // löst Hauptthread-Event aus
      dec(running);                           // runningflag zurücksetzen
      if FSkip then                           // Wenn geskipped wurde, Event neu setzen, für Restart
        FEvent.SetEvent();
      FSkip := False;
    end;
  end;

end;


//
// Interne ThreadEventFunktionen, die Events des Hauptthreads auslösen, falls assigned
//
procedure TGoogleWSThread.DoNotifyWork();
begin
  if Assigned( OnWork ) then OnWork( self, FCounter );
end;
procedure TGoogleWSThread.DoNotifyWorkBegin();
begin
  if Assigned( OnWorkBegin ) then OnWorkBegin( self, FCounter );
end;
procedure TGoogleWSThread.DoNotifyFinishDL();
begin
  if Assigned( OnFinishDownload ) then OnFinishDownload( self );
end;

Und Hauptunit

markieren

Delphi-Quelltext

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
69:
70:
71:
72:

type
  TForm1 = class(TForm)
    ListBox1: TListBox;
    Label1: TLabel;
    Button1: TButton;
    Label2: TLabel;
    procedure FormCreate(Sender: TObject);
    procedure ListBox1Click(Sender: TObject);
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
  private
    procedure OnThreadWork( Sender: TThread; count: Integer );
    procedure OnThreadWorkBegin( Sender: TThread; count: Integer );
    procedure AfterFinishedDownload( Sender: TObject );
  public
  end;

var
  Form1: TForm1;

implementation

uses SelfThread2;

{$R *.dfm}

procedure TForm1.FormCreate(Sender: TObject);
var
  i: Integer;
begin

  for i := 0 to 1000 do
    Listbox1.Items.Add( IntToStr( i ) );

  FSelfThread := TGoogleWSThread.Create(AfterFinishedDownload); //Thread inaktiv erzeugen
  FSelfThread.OnWork := OnThreadWork;
  FSelfThread.Resume;                                           //Thread aktivieren

end;


procedure TForm1.FormClose(Sender: TObject; var Action: TCloseAction);
begin
// So gehts
// Thread prüfen und ggf. terminieren und freigeben - Vielleicht läuft er ja noch
{  if Assigned( FSelfThread ) then begin
    with FSelfThread do begin
      Terminate;
      WaitFor;
    end;
    FreeAndNil( FSelfThread );
  end;
}
// so aber auch. TThread kümmert sich selber um .Terminate und .WaitFor
  If Assigned( FSelfThread ) and (FSelfThread <> nil) then
    FSelfThread.Free();
end;
procedure TForm1.ListBox1Click(Sender: TObject);
begin
  FSelfThread.DoDownload();  {SetEvent() im Thread setzen!}
end;
procedure TForm1.AfterFinishedDownload( Sender: TObject );
begin
  label2.Caption := 'Fertig: ' + IntToStr( FSelfThread.FTime );
end;
procedure TForm1.OnThreadWork( Sender: TThread; count: Integer );
begin
  label1.Caption := IntToStr( count );
end;
procedure TForm1.OnThreadWorkBegin( Sender: TThread; count: Integer );
begin
  label2.Caption := IntToStr( count );
end;

Bergmann89 · Verfasst: Fr 15.10.10 15:15

Hey,

die Events solltest du mit der Synchronize-Methodeaufrufen, das sie im Kontext der MainThreads ausgeführt werden. Denn wenn du das Event im Thread auslöst wird es auch vom Thread abgearbeitet. Wenn du jetzt im Event-Handler eine VLC-Komponente anspricht kann es zu komischen Fehlern kommen. Und die findest du nicht gleich, weil der Debuger auch nicht richtig damit klar kommt. Das hab ich nämlich schon gehabt un bin fast dran verzweifelt^^

MfG Bergmann

_________________
Ich weiß nicht viel, lern aber dafür umso schneller^^

jaenicke · Verfasst: Fr 15.10.10 16:30

Bergmann89 hat folgendes geschrieben :

die Events solltest du mit der Synchronize-Methodeaufrufen, das sie im Kontext der MainThreads ausgeführt werden

Das macht er schon genau richtig, sieh mal genauer hin. :zwinker:

Eine Sache fällt mir noch auf: Du benutzt Resume. Das kann manchmal Probleme machen. Besser: Den Thread fertig initialisieren, dann mit inherited sofort starten. Da du in Execute ohnehin wartest, passt das doch. ;-)

nru · Hält's aus hier Beiträge: 9 Erhaltene Danke: 1

Zitat:

Besser: Den Thread fertig initialisieren, dann mit inherited sofort starten.

Ungefähr so?

markieren

Unit Thread

1:
2:
3:
4:
5:
6:
7:
8:

constructor TGoogleWSThread.Create(Callback: TNotifyEvent);
begin
  FSkip := False;
  FEvent := TEvent.Create(nil,true,false,'');
  FreeOnTerminate := False;     {Release wird von MainThread initiiert}
  OnFinishDownload := Callback; {Eventfunktion des Hauptthreads}
  inherited Create( false );    {Thread startet sofort}
end;

markieren

Unit Main

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:

procedure TForm1.FormCreate(Sender: TObject);
var
  i: Integer;
begin

  for i := 0 to 1000 do
    Listbox1.Items.Add( IntToStr( i ) );

  FSelfThread := TGoogleWSThread.Create(AfterFinishedDownload); //Thread inaktiv erzeugen
  FSelfThread.OnWork := OnThreadWork;
  // FSelfThread.Resume;                                           //Thread aktivieren

end;

Dann werde ich mich jetzt dran machen, dies in die echte App. einzubinden.
Vielen Dank für Eure Anregungen! :zustimm:

Gruss
nru

	Mitgliederliste
	Gruppen
	Das Team
	Richtlinien
	Synonyme