TThread, irgendwas mache ich falsch

[KodeZwerg]

Aber das ist es doch, ich warte bis Handle nicht mehr gefunden wird (WAIT_FAILED)
Wie könnte ich es besser machen?

[Fritzew]

Aber wieso eigentlich?
Wenn der Thread fertig ist, das dem Mainthread mitteilen.
Da gibt es ja jede Menge Möglichkeiten. Aber pollen ist unnötig meiner Meinung nach.
Über Synchchronize oder eine Message wie auch immer.
Abbrechen kannst Du den HttpClient.Get ja sowieso nicht...

[KodeZwerg]

Also ProcessExplorer zeigt mir an das ein seperater Thread gestartet ist, wenn ich auf "Cancel Download" klicke wird "CancelThread := True;" gesetzt und der Thread ist verschwunden, meinst Du der lädt dann trotzdem noch weiter bzw wie könnte ich das unterbinden?
edit
Ok ich kann nun nachvollziehen was Du meinst und habe das Problem so gelöst:

Delphi-Quellcode:

MyThread.Terminate; // das hier (beschreibung sagt Thread arbeit sich erst ab)
Winapi.Windows.TerminateProcess(MyThread.Handle, 0); // mit dem ersetzt (das schliesst sofort eine Instanz)

//bzw jetzt die Friss oder Stirb methode
if not Winapi.Windows.TerminateProcess(MyThread.Handle, 0) then MyThread.Terminate;

[Fritzew]

Um Himmels willen........

Delphi-Quellcode:

//bzw jetzt die Friss oder Stirb methode
if not Winapi.Windows.TerminateProcess(MyThread.Handle, 0) then MyThread.Terminate;

Das ist jetzt aber nur noch falsch, schau mal in die Docu von TerminateProcess
https://msdn.microsoft.com/en-us/lib...(v=vs.85).aspx


Warum nicht einfach so etwas:
Form:

Delphi-Quellcode:

object Form5: TForm5
  Left = 0
  Top = 0
  Caption = 'Form5'
  ClientHeight = 168
  ClientWidth = 371
  Color = clBtnFace
  Font.Charset = DEFAULT_CHARSET
  Font.Color = clWindowText
  Font.Height = -11
  Font.Name = 'Tahoma'
  Font.Style = []
  OldCreateOrder = False
  OnCreate = FormCreate
  PixelsPerInch = 96
  TextHeight = 14
  object Label1: TLabel
    Left = 48
    Top = 32
    Width = 31
    Height = 13
    Caption = 'Label1'
  end
  object Button1: TButton
    Left = 48
    Top = 88
    Width = 249
    Height = 25
    Caption = 'Starte Thread'
    TabOrder = 0
    OnClick = Button1Click
  end
end

code:

Delphi-Quellcode:

unit Unit5;

interface

uses
   Winapi.Windows,
   Winapi.Messages,
   System.SysUtils,
   System.Variants,
   System.Classes,
   Vcl.Graphics,
   Vcl.Controls,
   Vcl.Forms,
   Vcl.Dialogs,
   Vcl.StdCtrls;

type
   TForm5 = class(TForm)
      Button1: TButton;
      Label1: TLabel;
      procedure Button1Click(Sender: TObject);
      procedure FormCreate(Sender: TObject);

   private
    { Private-Deklarationen }

      procedure StarteThread;
      procedure FinishTread(const Value: string);

   public
    { Public-Deklarationen }
   end;

var
   Form5: TForm5;

implementation

{$R *.dfm}

procedure TForm5.FormCreate(Sender: TObject);
begin
   Label1.Caption := '';
end;

procedure TForm5.Button1Click(Sender: TObject);
begin
   StarteThread;
end;

procedure TForm5.StarteThread;
Var temp : String;
begin
   Label1.Caption := 'Thread running';
   Button1.Enabled := False;
   TThread.CreateAnonymousThread(
      procedure
      begin
       try
         Sleep(5 * 1000); // Wait 5 secs
          temp := 'Ich habe fertig';
         // Oder halt Dein GetHttp
          //temp := GetHttp('wasauchimmer');


      // Und dem Mainthread mitteilen das wir etwas haben
      // Queue damit das erst passiert wenn der Mainthread wirklich Zeit hat.....
      if temp <> '' then
       TThread.Queue(nil,
            procedure
            begin
               FinishTread(temp);
            end);
       finally
           // Den Button wieder einschalten
          // Synchronize damit der Button sofort wieder enabled wird
         TThread.Synchronize(nil,
            procedure
            begin
               Button1.Enabled := true;
            end);
       end;

      end).Start;

end;

procedure TForm5.FinishTread(const Value: string);
begin
   Label1.Caption := Value;
end;

end.

[KodeZwerg]

Hallo Fritzew, Danke für Dein Beispiel, das funktioniert bei mir leider nicht so, Whookie gab mir bereits ähnlichen Code.

So sieht jetzt eine Funktion aus:

Delphi-Quellcode:

function TFormMain.GetTHTTPClient ( Const xURL : String ) : String;
var
 tmp : String;
 MyThread: System.Classes.TThread;
begin
  tmp := '';
  CancelThread := False;
  MyThread := System.Classes.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;
      end;
    end
  );
  MyThread.FreeOnTerminate := True;
  MyThread.Start;
  repeat
    Vcl.Forms.Application.ProcessMessages;
    System.SysUtils.Sleep(5);
    if CancelThread then
    begin
      CancelThread := False;
      tmp := 'Download aborted.';
      if not Winapi.Windows.TerminateProcess(MyThread.Handle, 0) then MyThread.Terminate;
    end;
  until ((tmp <> '') or (Winapi.Windows.WaitForSingleObject(MyThread.Handle, 5) = Winapi.Windows.WAIT_FAILED)); //  WAIT_FAILED = DWORD($FFFFFFFF);
  if tmp = '' then tmp := 'Error Occured @ '+xURL;
  Result := tmp;
end;

Aufgerufen wird diese Funktion mit einem Millisekundenzähler, gestoppt wird Zähler wenn Funktion beendet:

Delphi-Quellcode:

 Watch := System.Diagnostics.TStopWatch.Create();
 Watch.Start;
 if System.Length(Temp1) > 0 then
 DataString := GetTHTTPClient( Temp1 ); // <<<--- hier rufe ich Thread auf und warte bis er fertig ist damit ich DataString habe und weiter machen kann
 Watch.Stop;

Bisher kann ich keine Probleme entdecken.

[Fritzew]

Das Handling um den Thread abzubrechen ist, entschuldige bitte, nur Schrott.
Du kannst einen Thread nicht mit TerminateProcess beenden.
Ich verstehe nicht was dein Pollen da soll.
Aber es wurde eigentlich alles gesagt hier.
Meiner Meinung nach ist Dein Ansatz komplett 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