Problem mit TStringList

[Christian.Ossi]

Ich habe eine TStringList in einer Procedure vereinbart, die dort auch über Variable:=TStringList.Create erzeugt wird. Danach werden Dateinamen in die Liste geschrieben und die Liste an einen Thread übergeben, der die Dateien downloaden soll. Der Thread hat seine eigene TStringList und deshalb wird im Constructor die Variable übergeben.

Soweit sogut.

Der Thread macht was er soll und beendet sich am Ende und gibt seine eigene TStringList wieder frei, jedoch wenn ich am Ende der Procedure wenn alles schon abgeschlossen ist, den Befehl für die Freigabe der TStringList Variable (Variable.Free) einfüge, stürzt dort das Programm ab.


Quelltexte

Delphi-Quellcode:

Procedure KnopfUpdateClick(Sender: TObject);
Var Variable: TStringList;
Begin
Variable:=TStringList.Create;
Variable.Add(....);
HTTPThread:=HTTPDownload.Create(Variable);
While Not HTTPThread.Terminated Do Application.ProcessMessages;
Variable.Free; <--- ABSTURZ
End;

Delphi-Quellcode:

HTTPDownload = Class(TThread)
Protected
Procedure Execute; Override;
Private
HTTPSocket: TIdHTTP;
    Stream: TFileStream;
      Down: TStringList;
Public
Constructor Create(Const FDown: TStringList);
 Destructor Destroy; Override;
End;

Delphi-Quellcode:

Constructor HTTPDownload.Create(Const FDown: TStringList);
Begin
HTTPSocket:=TIdHTTP.Create(Nil);
Down:=TStringList.Create;
Down:=FDown;
Inherited Create(False);
End;

Destructor HTTPDownload.Destroy;
Begin
HTTPSocket.Free;
Down.Free;
Inherited;
End;

Procedure HTTPDownload.Execute;
Var I: Word;
Begin
FreeOnTerminate:=True;
If Not Terminated then
Begin
For I:=0 to (Down.Count - 1) Do
Begin
Stream:=TFileStream.Create(Down[I],fmCreate);
Try
HTTPSocket.Get(Down[I],Stream);
FreeAndNil(Stream);
Except
End;
End;
Destroy;
End;
End;

[xaromz]

Hallo,

Du machst gleich zwei Fehler:

Erstens (Dein angesprochenes Problem):

Delphi-Quellcode:

onstructor HTTPDownload.Create(Const FDown: TStringList); 
Begin 
HTTPSocket:=TIdHTTP.Create(Nil); 
Down:=TStringList.Create;
Down:=FDown;              // <--
Inherited Create(False); 
End;

In der markierten Zeile überschreibst Du die gerade erstellte TStringList-Instanz mit der übergebenen. Bedenke, dass "Down" nur ein Pointer auf ein Objekt ist. Damit zeigen also zwei Variablen auf das gleiche Objekt. Außerdem erzeugst Du ein Speicherleck.

Lösung:

Delphi-Quellcode:

onstructor HTTPDownload.Create(Const FDown: TStringList); 
Begin 
HTTPSocket:=TIdHTTP.Create(Nil); 
Down:=TStringList.Create; 
Down.Assign(FDown);       // <--
Inherited Create(False); 
End;

Zweitens:

Delphi-Quellcode:

Procedure KnopfUpdateClick(Sender: TObject); 
Var Variable: TStringList; 
Begin 
Variable:=TStringList.Create; 
Variable.Add(....); 
HTTPThread:=HTTPDownload.Create(Variable); 
While Not HTTPThread.Terminated Do Application.ProcessMessages; 
Variable.Free;
End;

Wozu ein Thread, wenn Du eh darauf wartest, dass er fertig wird? Du tust ja nach dem Beenden nichts mehr.
Überdenke da mal Dein grundsätzliches Design.

Gruß
xaromz

[Christian.Ossi]

Danke für erstens :)

Naja, da es sich um größere Dateien handelt, würde das Programm "hängen" - über den Thread mit ProcessMessages ist das Problem umgangen.

[xaromz]

Hallo,

Zitat:

Zitat von Christian.Ossi

Naja, da es sich um größere Dateien handelt, würde das Programm "hängen" - über den Thread mit ProcessMessages ist das Problem umgangen.

Wenn Du nur etwas im Thread abarbeiten willst, und der Thread unabhängig von Hauptprogramm ist (was er sein sollte), dann mach's doch so:

Delphi-Quellcode:

constructor HTTPDownload.Create(Const FDown: TStringList); 
Begin 
  inherited Create(True);  // <- suspendiert erzeugen
  HTTPSocket := TIdHTTP.Create(Nil); 
  Down := TStringList.Create; 
  Down.Assign(FDown);
End;

und

Delphi-Quellcode:

Procedure KnopfUpdateClick(Sender: TObject); 
Var 
  Variable: TStringList; 
Begin 
  Variable := TStringList.Create; 
  Variable.Add(....); 
  HTTPThread := HTTPDownload.Create(Variable); 
  HTTPThread.FreeOnTerminate := True;  // automatisch zerstören
  Variable.Free;  // aufräumen
  HTTPThread.Resume;  // los geht's
End;

Übrigens: In "Create" das inherited immer als Erstes aufrufen, in "Destroy" am Schluss.

//Edit:
Ich habe mir noch mal Deine "Execute"-Methode angesehen. Abgesehen von der Einrückung, die das Ganze sehr undurchsichtig macht, wie wäre es hiermit:

Delphi-Quellcode:

procedure HTTPDownload.Execute; 
var 
  I: Integer; // Integer statt Word
begin 
  I := 0;
  while (not Terminated) and (I < Down.Count) do // statt for-Schleife while-Schleife mit Abbruchbedingung
  begin
    Stream := TFileStream.Create(Down[I], fmCreate);
    try // try .. finally statt try .. except
      HTTPSocket.Get(Down[I], Stream);
    finally
      Stream.Free;
    end;
    Inc(I);
  end; 
end;

Gruß
xaromz