Einzelnen Beitrag anzeigen

Benutzerbild von jfheins
jfheins

Registriert seit: 10. Jun 2004
Ort: Garching (TUM)
4.579 Beiträge
 
#4

AW: Delphi einfaches Multithread Beispiel.

  Alt 12. Apr 2020, 21:33
Guten Abend
Zitat:
Ich werde jetzt sicher wieder von allen Seiten angemacht, weil ich nur am meckern bin. Aber das ist kein guter Beispielcode.
Zum Teil, aber zum Teil hast du mMn auch zu wenig kritisiert

Ich würde es grundlegend anders machen und Synchronize komplett rausschmeißen. Sendmessage/Postmessage und dann abarbeiten.
Also ich finde, Synchronize ist für ein kleines Beispiel schon OK. SendMessage/Postmessage wäre für mich schon eher fortgeschritten, weil die Datenübergabe komplexer ist.
Zitat:
Dann würde ich auch niemals einen Thread mit Terminate beenden.
Das finde ich schon i.O., weil das ja nur das Terminated flag setzt. Völlig harmlos, der Thread fragt das Flag ab und beendet sich bei Gelegenheit. Hat nichts mit dem WINAPI TerminateThread() zu tun.

Aber jetzt zum interessanten Teil - ich habe mal kommentiert:
Delphi-Quellcode:
procedure TmyThread.Execute;
var
  z: integer;
  cnt: integer;
begin
  inherited; // Unnötig
  SetLength(threads, length(threads)+1); // Gefährlich - SetLength ist NICHT thread-safe!
  threads[(high(threads))] := self.ThreadID;
  SYNCHRONIZE(Self.Synthreads);
  for z := 0 to 50 do
  begin
    inc(i); // Integer-zugriff ist atomar und damit i.O. (soweit ich weiß)
    sleep(random(500));
    Synchronize(Syni);
    if Terminated then
     break;
  end;
  for cnt:= low(threads) to High(threads) do
  if threads[cnt] = self.ThreadID then
   begin
      threads[cnt] := threads[high(threads)];
      SetLength(threads, length(threads)-1); // Auch hier eine race condition, mehrere Thread könnten SetLength gleichzeitig aufrufen!
     // Außerdem ein Speicherleck, der Thread wird nicht freigegeben
   end;
  Synchronize(Synthreads);
end;


procedure TForm1.Button2Click(Sender: TObject);
var i:integer;
begin
if Length(meinThread) >0 then // Überflüssig, die Schleife läuft ohne Elemente eh nicht
  for I := 0 to high(meinThread) do
    // Vergleich auf nil _aktuell_ überflüssig, da du es nie auf nil setzt. Zudem auch der check auf Terminated überflüssig - zweimal aufrufen schadet nicht.
    if ((meinThread[i] <> nil ) and (meinThread[i].Terminated = false)) then
     meinThread[i].Terminate;
end;
  Mit Zitat antworten Zitat