Hallo,

in einem Thread möchte ich eine Datei downloaden und in der Form der Anwendung den Fortschritt anzeigen.
Diese Anzeigeaktualisierung funktioniert nun in Rio mit dem Android-Device (Android 9) nicht mehr, erst nach Ende wird der letzte Stand angezeigt.

Was ist an meiner Implementierung falsch?

zusammenfalten · markieren

Delphi-Quellcode:

unit ThreadDownload;

interface

uses
  System.Classes, ......

type
  TDownloadThread = class(TThread)
    constructor Create;
    destructor Destroy; override;
  private
    FCS: TCriticalSection;
    IdFTP1: TIdFTP;
    progressvalue: int64;
    progressmax: int64;
    procedure IdFTPWorkBegin(ASender: TObject; AWorkMode: TWorkMode;
      AWorkCountMax: int64);
    procedure IdFTPWork(ASender: TObject; AWorkMode: TWorkMode;
      AWorkCount: int64);
    procedure IdFTPWorkEnd(ASender: TObject; AWorkMode: TWorkMode);
    procedure Updatelabel;
  protected
    procedure Execute(); override;
  public
    FileName: string;
    property CS: TCriticalSection read FCS;
  end;

var
  Thread_Download: TDownloadThread;

implementation

uses UnitUpdate;

constructor TDownloadThread.Create;
begin
  FreeOnTerminate := True;
  // Suspended starten
  inherited Create(True);
  try
    progressvalue := 0;
    progressmax := 1000;
    FCS := TCriticalSection.Create;

  except
    on E: Exception do
      LogAusgabe(E.ClassName + ' in TDownloadThread.Create : ' +
        E.Message, True);
  end;
end;

destructor TDownloadThread.Destroy;
begin
  try
    // was sonst noch zerstört werden muss
    FCS.DisposeOf;
    IdFTP1.DisposeOf;
    // globale Variable rücksetzen
    Thread_Download := nil;
  except
    on E: Exception do
      LogAusgabe(E.ClassName + ' in TDownloadThread.Destroy : ' +
        E.Message, True);
  end;
  inherited;
end;

procedure TDownloadThread.Execute;
var
  anz: integer;
begin
  anz := 0;
  try
    IdFTP1 := TIdFTP.Create(nil);
    IdFTP1.IPVersion := id_IPv4;
    IdFTP1.TransferType := ftBinary;
    IdFTP1.OnWork := IdFTPWork;
    IdFTP1.OnWorkBegin := IdFTPWorkBegin;
    IdFTP1.OnWorkEnd := IdFTPWorkEnd;

    IdFTP1.Host := FTPHost;
    IdFTP1.Username := FTPUser;
    IdFTP1.Password := DecryptFTP(FTPKey);
    IdFTP1.Passive := True;
    IdFTP1.Connect();
    // ist die gewünschte Datei überhaupt vorhanden?
    // wenn nein gibt Size -1 zurück!
    progressmax := IdFTP1.Size(FileName);
    if progressmax = -1 then
    begin
      IdFTP1.Disconnect;
      Terminate;
    end;
    try
      IdFTP1.Get(FileName, DownloadDir + FileName, True);
    except
      IdFTP1.Disconnect;
    end;
    Terminate;
  except
    on E: Exception do
      LogAusgabe(E.ClassName + ' in TDownloadThread.Execute : ' +
        E.Message, True);
  end;
end;

procedure TDownloadThread.IdFTPWorkBegin(ASender: TObject; AWorkMode: TWorkMode;
  AWorkCountMax: int64);
begin
  progressvalue := 0;
  Synchronize(Updatelabel);
end;

procedure TDownloadThread.IdFTPWork(ASender: TObject; AWorkMode: TWorkMode;
  AWorkCount: int64);
begin
  progressvalue := AWorkCount;
  Synchronize(Updatelabel);
end;

procedure TDownloadThread.IdFTPWorkEnd(ASender: TObject; AWorkMode: TWorkMode);
begin
  progressvalue := 0;
  Synchronize(Updatelabel);
end;

procedure TDownloadThread.Updatelabel;
begin
  FormUpdate.Updatelabel(progressvalue)
end;

end.

Und im Formular

markieren

Delphi-Quellcode:

procedure TFormUpdate.UpdateLabel(BytesDone: Integer);
begin
   Label1.Text := IntToStr(BytesDone);
end;

Hat jemand eine zündende Idee?

Danke
Ciao
Stefan

Ich habe bei mir in der Anwendung einen Timer in der UI laufen und aktualisiere alle 250ms dann die Progress-Bar/Label-Anzeige und setze im Thread dann nur den dementsprechenden Int-Wert. Damit spare ich mir alle synchronized-Aufrufe, welche in IdFTP1.OnWorkEnd etwas viele an der Anzahl sind und reduziere die Anzahl an UI-Updates auf einen sinnvollen Wert.

Hast du einmal ein Minimalbeispiel versucht? Sprich ein Thread, der einfach alle z.B. 500 Millisekunden die aktuelle Zeit in das Label schreibt?
Denn falls es am Gerät und/oder an der Delphiversion liegen sollte, wäre ein kleines komplettes Projekt zum Testen für uns hier gut.

Wie verwendest du die Threadklasse denn? (Ja, ich vermute mal, dass das passt, so wie es sich anhört, aber das können wir hier ja nicht wissen.)

@ philipp.hofmann
Hab ich das richtig verstanden? Dein thread schreibt einen Integer-Wert in eine globale Variable aus der sich die UI alle ca. 250msec den Wert für die Anzeige holt?

Da hast Du aber Glück, daß sich die konkurierenden Threads nicht ins Gehege kommen. Und Glück ist ein recht unzuverlässiger Bestandteil von Programmen.

Gruß
K-H

Zitat von p80286:

Da hast Du aber Glück, daß sich die konkurierenden Threads nicht ins Gehege kommen. Und Glück ist ein recht unzuverlässiger Bestandteil von Programmen.

Solange man atomare Funktionen zum Lesen und Setzen verwendet, ist eine Synchronisation oder ähnliches ohnehin nicht erforderlich.

Und solange man nur auf einer Seite Werte hineinschreibt und auf der anderen liest, passiert auch das nur mit je einem Assemblerbefehl, so dass das auch schon atomar ist. Vorausgesetzt ist dabei natürlich, dass es eine einfache 32-Bit Integervariable oder ein 32-Bit Integerfeld ist, das entsprechend ausgerichtet ist, z.B. durch Setzen von {$ALIGN 4}.

Wobei ich auch nie etwas offizielles dazu gefunden habe, nur Erfahrungswerte und Meinungen dass das so sein sollte wenn man sich grade mal konkreten Assemblercode unter 32 Bit anschaut (z.B. https://stackoverflow.com/q/5481030/2298252).

Grade wenn ich auch Win64 oder Mac oder iOS machen würde, dann wäre mir etwas Offizielles dazu echt lieber. Ich bin ein vorsichtiger Mensch, ich mache auch bei simplen Typen wie Integern Sperren drum, dann kann ich mir auch in Zukunft nichts vorwerfen.

Zitat von jaenicke:

Vorausgesetzt ist dabei natürlich, dass es eine einfache 32-Bit Integervariable oder ein 32-Bit Integerfeld ist, das entsprechend ausgerichtet ist, z.B. durch Setzen von {$ALIGN 4}.

Zitat von Der schöne Günther:

Grade wenn ich auch Win64 oder Mac oder iOS machen würde, dann wäre mir etwas Offizielles dazu echt lieber. Ich bin ein vorsichtiger Mensch, ich mache auch bei simplen Typen wie Integern Sperren drum, dann kann ich mir auch in Zukunft nichts vorwerfen.

Eben drum, das kann schiefgehen. Einen String als Byte-Speicher zu mißbrauchen war auch über Jahre oft geübte Praxis, Das geht auch nur noch wenn man ganz genau weiß was man treibt und wenn es schief geht ist man's selber schuld. Nicht zu empfehlen!

Gruß
K-H

Zitat von Der schöne Günther:

Ich bin ein vorsichtiger Mensch, ich mache auch bei simplen Typen wie Integern Sperren drum, dann kann ich mir auch in Zukunft nichts vorwerfen.

Dafür gibt es ja wie gesagt die atomaren Operationen. Die sind um Größenordnungen schneller als Sperren wie mit TMonitor.

Wieviele parallele Downloads laufen da denn? KISS ist es ja nun nicht unbedingt, wenn man für einen Schreiber und einen Leser mit Semaphoren oder ähnlichem Gedöns los legt. Klar ist Vorsicht die Mutter der Porzellankiste, aber...in diesem speziellen Fall ist Vorsicht die Mutter des Umständlichen.

Ganz abgesehen davon geht es ja überhaupt nicht.

Mein Programm macht x-Sachen parallel und wichtig ist, dass das abgespielte Video auch während eines parallelen Downloads flüssig weiterläuft. Daher verzichtet ich auf synchronized soweit es geht, sonst ruckelt das Video leider schnell. Daher dieses Vorgehen bei mir, wo es den Timer auch davor schon gab und ich ihn somit für die Fortschrittsanzeige mit nutze. Für eine Fortschrittsanzeige (die sich eh ständig aktualisiert und man ein Update auch mal auslassen kann), würde ich dann immer noch ein try-except einem ständig zuschlagenden synchronized vorziehen, wenn ich nicht atomare Werte nutze (was bei integer nicht der Fall ist). Ist aber sicherlich Ansichtssache und geht nur, wenn die Anzeige eh in der nächsten Sekunde wieder andere Werte zur Anzeige erhält.