TThread gibt beim Beenden Speicherleck

[DieDolly]

Stark vereinfacht habe ich folgende Threadklasse.
Erstellt wird die mit einem Aufruf von TTest.Run (passiert bei Programmstart und sieht schöner aus als irgendwas anonymes oder mit lokal deklarierten Klassenvariablen). Die private Deklaration von TestThread brauche ich extern nicht, deswegen private.

Das Sleep 10 Sekunden ist zum Testen da.
Wenn ich das Programm innerhalb der ersten 10 Sekunden schließe, bekomme ich einen Memory Leak.

Wie lässt sich das beheben, ohne am grundlegenden Aufbau der Klasse was zu ändern?
Der Thread soll bei Programmstart erzeugt werden aber erst noch warten, bis er seine Arbeit macht.

Delphi-Quellcode:

unit UTestThread;

interface

uses
 System.Classes, System.SysUtils;

type
 TTestThread = class(TThread)
 private
  {Private-Deklarationen}
 public
  {Public-Deklarationen}
  constructor Create;
 protected
  procedure Execute; override;
 end;

type
 TTest = record
 private
  class var TestThread: TTestThread;
 public
  class procedure Run; static;
 end;

implementation

constructor TTestThread.Create;
begin
 inherited Create;

 FreeOnTerminate := True;
end;

class procedure TTest.Run
begin
 TestThread := TTestThread.Create;
end;

procedure TTestThread.Execute;
begin
  Sleep(10000);
  
  GewinneImLotto;
end;

end.

[Zacherl]

Es gibt die Methode

Delphi-Quellcode:

Thread.WaitFor()

, die du verwenden kannst, um vor Terminierung des Programms auf das Ende des Threads zu warten.

[DieDolly]

Kann man den Thread auch einfach terminieren?
Ich dachte an sowas. Aber das gibt wieder eine Fehlermeldung, wenn der Thread nicht mehr existiert.

Sollten diese beiden Vergleiche nicht eigentlich greifen, wenn der Thread schon lange fertig ist?

Delphi-Quellcode:

// FormDestroy

if Assigned(TTestThread.TestThread) and (TTestThread.TestThread <> nil) then
  TTestThread.TestThread.Terminate;

Zitat:

'Ein extern erstellter Thread kann nicht beendet werden'

(Thread ist schon beendet und diese Meldung kommt im FormDestroy).

[Zacherl]

Delphi-Quellcode:

.Terminate()

solltest du grundlegend immer vermeiden. Das erzeugt nämlich seinerseits Memory Leaks, wenn der Thread noch Speicher reserviert hat, den er nicht mehr freigeben kann. Ist beim Programmende allerdings im Grunde so oder so egal. Deine Bedingung trifft nicht zu, da

Delphi-Quellcode:

.FreeOnTerminate := true

zwar den Thread freigibt, nicht aber die Referenz (deine Variable) auf

Delphi-Quellcode:

nil

setzt.

Ich persönlich würde auf

Delphi-Quellcode:

.FreeOnTerminate := true

verzichten und beim Programmende (z.B im

Delphi-Quellcode:

OnClose

des MainForms) die

Delphi-Quellcode:

.WaitFor()

Methode nutzen und danach den Thread selbstständig mit

Delphi-Quellcode:

.Free()

freigeben. Du kannst zusätzlich auch noch

Delphi-Quellcode:

Thread.Terminated()

überschreiben und dort eine private Statusvariable setzen. Für den Fall, dass der Thread Aufgaben in einer Schleife bearbeitet, kannst du dann dieses interne Feld als zusätzliche Schleifenbedingung nutzen.

[DieDolly]

Zitat:

Deine Bedingung trifft nicht zu,

Wenn ich in die Bedingung ein showmessage('X') schreibe, wird mir das bei Programmende angezeigt, obwohl der Thread schon komplett durch ist.

[Zacherl]

Zitat:

Zitat von DieDolly (Beitrag 1426806)

Zitat:

Deine Bedingung trifft nicht zu,

Wenn ich in die Bedingung ein showmessage('X') schreibe, wird mir das bei Programmende angezeigt, obwohl der Thread schon komplett durch ist.

Ja, genauer gesagt trifft die Bedingung immer zu. Gründe siehe einen Post weiter oben.

[DieDolly]

Idee zu WaitFor:
ich setze von Extern ein Flag das sagt, dass das Sleep unterbrochen werden kann. Das werte ich dann hier aus

Delphi-Quellcode:

 i := 0;

   while i < 5000 do
    begin
     Inc(i);
     Sleep(1);

     if FlagXYZ then
      Break;
    end;

Dann WaitFor und Free. Wäre das eine Möglichkeit?

[Zacherl]

Ja genau, so in der Art. Mit dem Überschreiben von

Delphi-Quellcode:

.Terminated

geht es noch ein wenig eleganter. Dann musst du beim Programmende

Delphi-Quellcode:

.Terminate(); .WaitFor(); .Free();

aufrufen. Das

Delphi-Quellcode:

FreeOnTerminate

darfst du dann aber auf keinen Fall verwenden, weil es sonst wieder sein kann, dass der Thread bereits automatisch zerstört wurde.

[DieDolly]

Muss bei überschriebenen Terminate das inherited dann raus? Weil sonst gibt es ja wieder die Probleme die du erwähntest.

Delphi-Quellcode:

 public
  procedure Terminate; // nicht virtuell
 end;

procedure TTestThread.Terminate;
begin
 inherited;

 BreakSleep := True;
end;

Edit
klappt bis jetzt wunderbar. Das Problem im Großen und Ganzen war tatsächlich FreeOnTerminate.

[HolgerX]

Hmm..

Zitat:

Zitat von Zacherl (Beitrag 1426805)

Delphi-Quellcode:

.Terminate()

solltest du grundlegend immer vermeiden. Das erzeugt nämlich seinerseits Memory Leaks, wenn der Thread noch Speicher reserviert hat, den er nicht mehr freigeben kann. Ist beim Programmende allerdings im Grunde so oder so egal. Deine Bedingung trifft nicht zu, da

Delphi-Quellcode:

.FreeOnTerminate := true

zwar den Thread freigibt, nicht aber die Referenz (deine Variable) auf

Delphi-Quellcode:

nil

setzt.

Ich persönlich würde auf

Delphi-Quellcode:

.FreeOnTerminate := true

verzichten und beim Programmende (z.B im

Delphi-Quellcode:

OnClose

des MainForms) die

Delphi-Quellcode:

.WaitFor()

Methode nutzen und danach den Thread selbstständig mit

Delphi-Quellcode:

.Free()

freigeben. Du kannst zusätzlich auch noch

Delphi-Quellcode:

Thread.Terminated()

überschreiben und dort eine private Statusvariable setzen. Für den Fall, dass der Thread Aufgaben in einer Schleife bearbeitet, kannst du dann dieses interne Feld als zusätzliche Schleifenbedingung nutzen.

Aus meiner Sicht ist das Blödsinn!!!

Zitat aus der Delphi Hilfe (hier D6):

Zitat:

TThread.Terminate

Die Methode Terminate signalisiert die Beendigung eines Threads. Dazu wird die Eigenschaft Terminated auf True gesetzt.

procedure Terminate;

Beschreibung

Terminate setzt die Eigenschaft Terminated des Thread auf True und signalisiert, daß der Thread so bald wie möglich beendet werden sollte. Im Gegensatz zu der Windows-API-Methode TerminateThread, die den Thread sofort beendet, fordert die Methode Terminate den Abbruch des Thread nur an. Dadurch kann der Thread vor seiner Beendigung noch erforderliche Aufräumarbeiten durchführen.

Die Methode Execute eines Threads sowie alle von Execute aufgerufenen Methoden sollten die Eigenschaft Terminated regelmäßig abfragen und die Ausführung gegebenenfalls beenden.



TThread.Terminated

Die Eigenschaft Terminated zeigt an, ob der Thread eine Aufforderung zum Beenden erhalten hat.

property Terminated: Boolean;

Beschreibung

Die Methode Execute des Threads sollte ebenso wie alle Methoden, die von Execute aufgerufen werden, die Eigenschaft Terminated abfragen und gegebenenfalls die Ausführung beenden. Die Methode Terminate setzt die Eigenschaft Terminated auf True.

Die Methode Terminate bietet ein Verfahren, einen Thread kontrolliert abzubrechen, wobei allerdings die Kooperation des Execute-Codes Voraussetzung ist.

Somit braucht hier nichts 'Überschrieben' werden, weil TThread.Terminate mit der Abfrage nach 'Terminated' in der Execute Methode genau das macht was notwendig ist!

Somit statt 'FreeOnTerminate := True;' einzubauen, einfach das 'Terminated' abfragen...

Zum Beenden eines Threads im Destroy (ohne FreeOnTerminate := True) somit das

Delphi-Quellcode:

Thread.Terminate;  // Dem Thread 'signalisieren' dass er sich beenden soll (Sprich alles in Execute beenden und raus)
Thread.WaitFor;    // Warten, bis Execute abgeschlossen
Thread.Free;       // Thread freigeben..

[Schokohase]

Dafür gibt es schon eine extra Methode

Delphi-Quellcode:

Thread.Terminate; // Dem Thread 'signalisieren' dass er sich beenden soll (Sprich alles in Execute beenden und raus)
Thread.WaitFor; // Warten, bis Execute abgeschlossen
Thread.Free; // Thread freigeben..

nämlich

Delphi-Quellcode:

Thread.Free;

- ja, so einfach

[Zacherl]

Zitat:

Zitat von Schokohase (Beitrag 1426816)

Dafür gibt es schon eine extra Methode [..] - ja, so einfach

Oh, ist das schon immer so gewesen?

@HolgerX:
Du hast recht, es war auch nicht die

Delphi-Quellcode:

.Terminated()

Methode, sondern

Delphi-Quellcode:

.TerminatedSet()

oder sowas in der Art. Kann leider grade nicht nachschauen. Hintergrund des Überschreibens war, dass meine Threads oft auf Events reagieren und die entsprechende Wait Methode blockiert ja. Im

Delphi-Quellcode:

.TerminatedSet()

habe ich dann noch das Event signaled. Da vorher dann auch Terminated gesetzt wurde, kann ich meine Schleife dann direkt beenden und muss nicht erst warten, bis das Event "zufällig" wieder auslöst.

[mjustin]

Zitat:

Zitat von Schokohase (Beitrag 1426816)

Dafür gibt es schon eine extra Methode
nämlich

Delphi-Quellcode:

Thread.Free;

- ja, so einfach

Seit ich diesen Hinweis gelesen habe bin ich kein Freund von TThread.Free mehr:

Zitat:

NEVER call Free() on a thread that has not been signaled to terminate yet. DO NOT rely on the thread destructor calling Terminate() and WaitFor() for you. ALWAYS call them explicitally in your own code instead. Plenty of people have run into problems over the years by freeing a running thread and letting the destructor do the termination. It just does not work correctly, and is not the right place to handle this anyway. – Remy Lebeau

[peterbelow]

Zitat:

Zitat von DieDolly (Beitrag 1426802)

Stark vereinfacht habe ich folgende Threadklasse.
Erstellt wird die mit einem Aufruf von TTest.Run (passiert bei Programmstart und sieht schöner aus als irgendwas anonymes oder mit lokal deklarierten Klassenvariablen). Die private Deklaration von TestThread brauche ich extern nicht, deswegen private.

Das Sleep 10 Sekunden ist zum Testen da.
Wenn ich das Programm innerhalb der ersten 10 Sekunden schließe, bekomme ich einen Memory Leak.

Wie lässt sich das beheben, ohne am grundlegenden Aufbau der Klasse was zu ändern?
Der Thread soll bei Programmstart erzeugt werden aber erst noch warten, bis er seine Arbeit macht.

Du mußt auf eine Art und Weise warten, die es erlaubt, den Thread auch aufzuwecken, wenn er sich vorzeitig beenden soll.
Das klassische Konstrukt dafür verwendet einen TSimpleEvent. Den erzeugst Du im Constructor der Thread-Klasse bevor Du den inherited constructor aufrufst (nur wichtig in alten Delphi-Versionen). In der Execute-Methode wartest Du dann auf den Event, in deinem Fall mit einem Timeout, der deiner Sleep-Zeit entspricht. Dann überschreibst Du Terminate als

inherited;
WaitEvent.setEvent;

Der Rückgabewert von TSimpleEvent.Waitfor sagt Dir, ob die Methode zurückgekehrt ist weil der Event ausgelöst wurde oder der Timeout abgelaufen ist.