TThread, irgendwas mache ich falsch

[himitsu]

Zitat von Fritzew:

Meiner Meinung nach ist Dein Ansatz komplett falsch

Ich nenne sowas fahrlässig.

Delphi-Quellcode:

MyThread.FreeOnTerminate := True;
MyThread.Start;
... 
if TerminateProcess(MyThread.Handle, 0) then MyThread.Terminate;

Ab "..." darf von außen niewieder niemals nicht auf diese Variable zugegriffen werden!

Denn wenn der Thread endet, wird das Objekt automatische gelöscht und ein Zugriff ist nicht mehr möglich.

Lösung: FreeOnTerminate:=False; und am Ende ein manuelles Free,
oder von innerhalb des Threads nach außen den Zustand in einer weiteren Variable/Event/Sonstwas speichern/informieren.


TerminateProcess schießt den ganzen Prozess ab, also alle Threads, (rate mal, warum diese API so heißt, wie sie heißt)
aber da hier auch noch ein falsches Handle übergeben wurde, und der Entwickler fahrlässig nicht alle Rückgabewerte auswertet (GetLastError), bekommt er das nicht mit.

Man sagt dem Thread er soll sich beenden (Variable/Event, wie z.B. TThread.Terminate) und innerhalb des Threads beendet dieser sich definiert/kontrolliert selber.

Und dass man Prozesse, abe vor allem Threads niemals hart abschießen darf, sollte jedem klar sein, wenn er endlich sich richtig mit Treads beschäftigen würde. (Tutorials gelesen und verstanden?)
Der Thread, bzw. die durch ihn verwalten Objekte/Speicher bleiben so in einem undefinierten Zustand und können den kompletten Prozess lahm legen,
wenn du den Thread abschießt, während er gerade beim Speichermanager etwas anfordert/freigibt, also z.B. zwischen dem Sperren und Freigeben einer CriticalSection, dann bleibt jene für immer gesperrt und auch andere Threads können nicht mehr ihren Speicher verwalten und bleiben somit hängen.


So, nun wurde aber wirklich schon alles mehrfach erwähnt und ich bin raus aus dem Thema.
Wünsche euch noch viel Spaß.

[Fritzew]

Mein letzter Beitrag dazu...
himitsu hat vollkommen recht.


Hier noch mal eine abgewandelte Version:

Delphi-Quellcode:

procedure TForm5.StarteThread;
Var temp : String;
    Watch : TStopwatch;
begin
   Label1.Caption := 'Thread running';
   Button1.Enabled := False;
   Watch := TStopWatch.Create();
   Watch.Start;

   TThread.CreateAnonymousThread(
      procedure
      begin
       try
         Sleep(5 * 1000); // Wait 5 secs
          temp := 'Ich habe fertig';
         // Oder halt Dein GetHttp
          //temp := GetHttp('wasauchimmer');

       finally
         TThread.Synchronize(nil,
            procedure
            begin
              Watch.Stop;
               FinishTread(temp, Watch.ElapsedMilliseconds);
               Button1.Enabled := true;
            end);
       end;

      end).Start;

end;

procedure TForm5.FinishTread(const Value: string; const Millisecs : int64);
begin
   Label1.Caption := format('Message: %s  time in Miilsecs  %d',[Value, Millisecs]) ;
end;

[KodeZwerg]

Ok Danke nochmal für die vielen Warnungen, ich hatte eh das falsche hier her kopiert,
if CancelThread then if not Winapi.Windows.TerminateThread(MyThread.Handle, 0) then MyThread.Terminate; so war es.

Ich habe nun die zweite Code Variante erfolgreich umgesetzt

Hier mein jetziger Code:

Delphi-Quellcode:

procedure TFormMain.GetTHTTPClient ( Const xURL : String );
Var tmp : String;
    Watch : TStopwatch;
begin
  Watch := TStopWatch.Create();
  Watch.Start;
  TThread.CreateAnonymousThread(
   procedure
   var
     HttpClient: System.Net.HttpClient.THttpClient;
     HttpResponse: System.Net.HttpClient.IHttpResponse;
   begin
      HttpClient := System.Net.HttpClient.THTTPClient.Create;
      try
        HttpClient.UserAgent := 'Mozilla/4.0 (compatible; MSIE 7.0; Windows; U; Windows NT 5.2; Trident/4.0; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.04506.30; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3 */*';
        HttpClient.MaxRedirects := 10;
        HttpClient.HandleRedirects := True;
        HttpClient.ContentType := '*/*';
        HttpClient.Accept := '*/*';
        HttpClient.ResponseTimeout := 5000;
        HttpClient.ConnectionTimeout := 5000;
        try
          HttpResponse := HttpClient.Get( xURL );
          tmp := HttpResponse.ContentAsString();
        except
          on e: System.SysUtils.Exception do
            tmp := 'Error Occured @ '+xURL+' - '+e.Message;
        end;
      finally
        HttpClient.Free;
        TThread.Synchronize(nil,
         procedure
         begin
           Watch.Stop;
           FinishTread(xURL, tmp, Watch.ElapsedMilliseconds);
          end);
      end;
    end
  ).Start;
end;

procedure TFormMain.FinishTread( Const sUrl, sData: String; Const Millisecs : Int64 );
var
 i: Integer;
begin
  DataString := sData;
  if System.Length(DataString) > 0 then
  begin
    MemoText.Lines.Text := DataString;
    i := System.Length(MemoText.Lines.Text) ;
    MemoText.Lines.Add('');
    MemoText.Lines.Add('HTTP/S HTML Source from: '+sURL);
    if System.Length(DataString)-i < 0 then MemoText.Lines.Add('Downloaded: '+System.SysUtils.IntToStr(System.Length(DataString)) +' bytes, displaying: ' +System.SysUtils.IntToStr(i)+ ' chars. Additional added '+System.SysUtils.IntToStr(i-Length(DataString))+' extra Unicode bytes.');
    if System.Length(DataString)-i = 0 then MemoText.Lines.Add('Downloaded: '+System.SysUtils.IntToStr(System.Length(DataString)) +' bytes, displaying: ' +System.SysUtils.IntToStr(i)+ ' chars. Plain Ascii detected.');
    if System.Length(DataString)-i > 0 then MemoText.Lines.Add('Downloaded: '+System.SysUtils.IntToStr(System.Length(DataString)) +' bytes, displaying: ' +System.SysUtils.IntToStr(i)+ ' chars. Warning! '+System.SysUtils.IntToStr(System.Length(DataString)-i)+' bytes missing in Display!');
    if CheckBoxBenchmark.Checked then
    begin
      if (((ComboBoxBitsBytes.ItemIndex = 0)or(ComboBoxBitsBytes.ItemIndex = 1))and((ComboBoxByteCalc.ItemIndex = 0)or(ComboBoxByteCalc.ItemIndex = 1))) then
        MemoText.Lines.Add('Downloaded needed '+System.SysUtils.IntToStr(Millisecs)+' ms, that is '+System.SysUtils.FloatToStrF(Length(DataString) / (Millisecs / 1000), ffFixed, 35, 2)+' bytes/second <-> '+System.SysUtils.FloatToStrF((Length(DataString) / 1024) / (Millisecs / 1000), ffFixed, 35, 2)+' kbyte/s <-> '+System.SysUtils.FloatToStrF((Length(DataString) / 1024 / 1024) / (Millisecs / 1000), ffFixed, 35, 2)+' mbyte/s.');
      if (((ComboBoxBitsBytes.ItemIndex = 0)or(ComboBoxBitsBytes.ItemIndex = 2))and((ComboBoxByteCalc.ItemIndex = 0)or(ComboBoxByteCalc.ItemIndex = 1))) then
        MemoText.Lines.Add('Downloaded needed '+System.SysUtils.IntToStr(Millisecs)+' ms, that is '+System.SysUtils.FloatToStrF((Length(DataString)*8) / (Millisecs / 1000), ffFixed, 35, 2)+' bits/second <-> '+System.SysUtils.FloatToStrF(((Length(DataString)*8) / 1024) / (Millisecs / 1000), ffFixed, 35, 2)+' kbit/s <-> '+System.SysUtils.FloatToStrF(((Length(DataString)*8) / 1024 / 1024) / (Millisecs / 1000), ffFixed, 35, 2)+' mbit/s.');
      if ComboBoxByteCalc.ItemIndex = 0 then
        MemoText.Lines.Add('Above calculations based on 1024 byte = 1 kb for your pleasure 1000 byte = 1 kb follows.');
      if (((ComboBoxBitsBytes.ItemIndex = 0)or(ComboBoxBitsBytes.ItemIndex = 1))and((ComboBoxByteCalc.ItemIndex = 0)or(ComboBoxByteCalc.ItemIndex = 2))) then
        MemoText.Lines.Add('Downloaded needed '+System.SysUtils.IntToStr(Millisecs)+' ms, that is '+System.SysUtils.FloatToStrF(Length(DataString) / (Millisecs / 1000), ffFixed, 35, 2)+' bytes/second <-> '+System.SysUtils.FloatToStrF((Length(DataString) / 1000) / (Millisecs / 1000), ffFixed, 35, 2)+' kbyte/s <-> '+System.SysUtils.FloatToStrF((Length(DataString) / 1000 / 1000) / (Millisecs / 1000), ffFixed, 35, 2)+' mbyte/s.');
      if (((ComboBoxBitsBytes.ItemIndex = 0)or(ComboBoxBitsBytes.ItemIndex = 2))and((ComboBoxByteCalc.ItemIndex = 0)or(ComboBoxByteCalc.ItemIndex = 2))) then
      MemoText.Lines.Add('Downloaded needed '+System.SysUtils.IntToStr(Millisecs)+' ms, that is '+System.SysUtils.FloatToStrF((Length(DataString)*8) / (Millisecs / 1000), ffFixed, 35, 2)+' bits/second <-> '+System.SysUtils.FloatToStrF(((Length(DataString)*8) / 1000) / (Millisecs / 1000), ffFixed, 35, 2)+' kbit/s <-> '+System.SysUtils.FloatToStrF(((Length(DataString)*8) / 1000 / 1000) / (Millisecs / 1000), ffFixed, 35, 2)+' mbit/s.');
    end;
  end;
  ButtonDownload.Enabled := True;
end;

procedure TFormMain.ButtonDownloadClick(Sender: TObject);
begin
  ButtonDownload.Enabled := False;
  MemoText.Clear;
  MemoText.Lines.Add('Downloading Data from ' +Temp1);
  MemoText.Lines.Add('Please Wait...');
  GetTHTTPClient( 'https://www.google.com/' ); // hier startet nun der thread und macht sein ding bis er fertig ist.
end;

Danke sehr! Funktioniert soweit so gut, jetzt meine Frage, wie kann ich Download abbrechen?
Meine Methode mit einem Boolean als Trigger war ja die Falsche und erst recht der Befehl zum beenden.

[p80286]

@Himitsu
Danke! in 13 Zeilen das wichtigste geschrieben und trotzdem, die meisten Tutorials eiern da nur herum.
Einem Thread gibt man alles notwendige mit und läßt ihn dann seine Aufgabe erledigen. Und wenn aus welchen Gründen auch immer, er zwischenzeitlich neue Instruktionen benötigt, dann hat der Programmierer ganz tolle Arbeit abgeliefert.
Das ist alles andere als simpel, und das macht man nicht im vorübergehen.

Gruß
K-H

[Zacherl]

Zitat von KodeZwerg:

jetzt meine Frage, wie kann ich Download abbrechen?

Abbruchbedingung wird immer im Thread selbst geprüft. Dazu musst du die IdHTTP Komponente allerdings erstmal dazu bringen deinen Download in Chunks aufzusplitten (bzw. macht die Komponente das tatsächlich sowieso schon; du könntest das OnWork Event zuweisen und dort auf TThread.Terminated prüfen).

Zum Terminieren selbst rufst du dann TThread.Terminate auf. Allerdings solltest du in diesem Fall dann von der selbstständigen Freigabe (FreeOnTerminate ) absehen.

[KodeZwerg]

Danke Zacherl, ich glaube ich Verstehe in welche Richtung Du mich bringen magst!
System.Net.HttpClient.THTTPClient.OnReceiveData da könnte/müßte ich glaube ich ansetzen und dann noch gucken wie ich disconnect hinbekomme (ich vermute darauf warst Du aus?).
Wenn ich es wie zuletzt gezeigt in einer Prozedur mit createthread(hier der threadcode).Start aufrufe, ist das dann wie FreeOnTerminate := True?
Wenn ich eh schon dabei bin stell ich alles auf Stream statt String um, dann lohnt sich der ganze Umbau um so mehr und ich glaube das ist Voraussetzung um mit OnReceiveData zu arbeiten, das wiederum ermöglicht so etwas wie eine ProgressBar(). Ich lese mich mal mehr in den THTTPClient mehr rein, der ist echt Umfangreich aber kann auch simpel (wie bei mir) bedient werden, nur um das zu machen was ich will muss es eben komplexer werden, danke auf jeden Fall für den stuppser mit Indy.OnWork Event!