Wieso bekomme ich hier einen Deadlock?

[Zacherl]

Hey,

ich habe hier ein etwas anspruchsvolleres Problem: Ich habe einen Thread, welcher praktisch dauerhaft (mit Sleep(10) unterbrochen) auf eine ThreadList zugreift (LockList und UnlockList in jedem Zyklus). Nun habe ich eine Benutzerfunktion, die von der VCL Oberfläche aus ebenfalls auf die ThreadList zugreift. Dies geschieht sehr selten.
Der Thread selbst ruft bei jedem Durchgang mehrmals Ereignissfunktionen, in diesem Falle OnProgress, auf.

Soweit funktioniert alles wunderbar. Die Synchronisation der Threads arbeitet gut und durch das Sleep findet auch nach jedem Zyklus ein Context Switch statt. Jetzt möchte ich aber im OnProgress Event eine Fortschrittsanzeige aktualisieren. Da das Event im Context des Threads läuft, muss ich hier mit der VCL synchronisieren. Hierzu benutze ich der Einfachheit halber die AsyncCalls Unit (allerdings auch mit Synchronize() des Threads getestet).

Okay der Thread läuft nun und der Fortschritt wird auch wunderbar angezeigt. ABER: Rufe ich jetzt die Benutzerfunktion auf, kommt es zum Deadlock, weil LockList komischerweise bis in alle Ewigkeiten auf die Synchronisation wartet. Dieser Fehler tritt wie gesagt nur auf, wenn ich im Event mit der VCL synchronisiere. :?

Momentan habe ich das Problem jetzt so behoben, dass ich vor Aufruf des Events aus dem Thread heraus UnlockList aufrufe und nach dem Event direkt wieder LockList. Das funktioniert zwar, mich würde aber trotzdem interessieren warum es anders zum Deadlock kommt.

Viele Grüße
Zacherl

[SirThornberry]

Also in deinem Quelltext kann ich keinen Fehler sehen :wink:

[Zacherl]

Hier mal ein paar Teile Code. Der besagte Thread:

Delphi-Quellcode:

procedure TdxIDTPSendThread.Execute;
var
  I, X, Y: Integer;
  List: TList;
  Transfer: TdxIDTPTransfer;
begin
  while (not Terminated) do
  begin
    List := FWriter.TransferList.LockList;
    try
      X := 0;
      Y := 0;
      for I := 0 to List.Count - 1 do
      begin
        //
        // !! Code gekürzt: An dieser Stelle wird das OnProgress Event aufgerufen !!
        //
        if (not Transfer.Priority) then
        begin
          Inc(X);
        end;
        Inc(Y);
      end;
      // Beendete Transfers entfernen
      for I := List.Count - 1 downto 0 do
      begin
        Transfer := TdxIDTPTransfer(List[I]);
        if (Transfer.TransferState = tsFinished) then
        begin
          List.Delete(I);
          Transfer.Free;
        end;
      end;
    finally
      FWriter.TransferList.UnlockList;
    end;
    Sleep(10);
    // Thread suspendieren
    if (Y = 0) then
    begin
      {$WARNINGS OFF}
      Suspend;
      {$WARNINGS ON}
    end;
  end;
end;

Die Funktion, die von der GUI aus aufgerufen werden kann:

Delphi-Quellcode:

function TdxIDTPWriter.SendFile(Filename: String; Offset: Int64;
  TransferCode: Word; Encrypt: Boolean; const Compress, Priority: Boolean;
  const CreateSuspended: Boolean;
  const PacketSize: TdxPacketSize): TdxIDTPTransfer;
var
  List: TList;
begin
  Result := TdxIDTPFileTransfer.Create(FSendThread, Filename, Offset,
    GenerateTransferID, TransferCode, Priority, Encrypt, Compress, PacketSize);
  List := FTransferList.LockList;
  try
    //
    // !! Hier kommt es zum Deadlock !!
    //
    List.Add(Pointer(Result));
  finally
    FTransferList.UnlockList;
  end;
  if (CreateSuspended) then
  begin
    Result.Suspend;
  end else if (FSendThread.Suspended) then
  begin
    {$WARNINGS OFF}
    FSendThread.Resume;
    {$WARNINGS ON}
  end;
end;

Mein Code im Event:

Delphi-Quellcode:

var
  Item: TListItem;
begin
  AsyncCalls.EnterMainThread;
  try
    Item := TListItem(Transfer.Data);
    Item.SubItems[1] := IntToStr(Progress);
  finally
    AsyncCalls.LeaveMainThread;
  end;

Lasse ich den Code im Event beispielsweise ganz weg oder verzichte Testweise auf die Synchronisierung kommt es beim Aufruf von SendFile nicht zum Deadlock. Mit der Synchronisation allerdings schon.

[himitsu]

Kann zwar nichts entdecken, aber ein Deadlock entstecht z.B. durch sowas:

> in einem Thread wird die Liste gesperrt
> Verarbeitug wird im anderem Thread forgesetzt (Synchronize oder AsyncCalls)
> im anderem Thread soll ebenfalls die Liste gesperrt werden
(dieses geschieht niemals, da z.B. eine CriticalSection sich den sperrenden Thread merkt und somit hier auf die Freigabe im ersten Thread gewartet wird)
...

[Zacherl]

Ja deshalb war ich auch etwas verwundert, weil eigentlich ja alles funktionieren sollte. Kann mir höchstens vorstellen, dass das schnelle LockList und UnlockList im Thread da irgendwas durcheinanderbringt. :gruebel:

[H4ndy]

Fuehre mal einen Lock-Counter in die Threadlist ein und schau einfach, was der fuer nen Wert hat. Bzw. einfach bei einem Wert > 1 ne Assertion einbauen oder ne Exception werfen o.ae.

[Zacherl]

Okay die Exception vom Counter wird nie geworfen. Die ThreadList selbst scheint nicht das Problem zu sein. Der Code in meinem Progress Event war vorher so:

Delphi-Quellcode:

  Item := TListItem(Transfer.Data);
  TAsyncCalls.VCLSync(procedure
  begin
    Item := TListItem(Transfer.Data);
    Item.SubItems[1] := IntToStr(Round(Transfer.CurrentDataSize /
      Transfer.TotalDataSize * 100));
    Item.SubItems[2] := IntToStr(Round(100 - (Transfer.CompressedDataSize /
      Transfer.CurrentDataSize * 100)));
  end);

Das Problem war scheinbar, dass die Anzeige einfach zu schnell aktualisiert wurde (jede 8KiB). Hab dann jetzt zufällig den Code geändert, damit das ListView nicht mehr flackert und jetzt ist auch mein Deadlock weg :wall: Hier ist der neue Code:

Delphi-Quellcode:

Progress := Round(Transfer.CurrentDataSize / Transfer.TotalDataSize * 100);
  Item := TListItem(Transfer.Data);
  if (Integer(Item.Data) < Progress) then
  begin
    Item.Data := Pointer(Progress);
    TAsyncCalls.VCLSync(procedure
    begin
      Item := TListItem(Transfer.Data);
      Item.SubItems[1] := IntToStr(Round(Transfer.CurrentDataSize /
        Transfer.TotalDataSize * 100));
      Item.SubItems[2] := IntToStr(Round(100 - (Transfer.CompressedDataSize /
        Transfer.CurrentDataSize * 100)));
    end);
  end;

Vom verstehen dieses Problems bin ich zwar noch meilenweit entfernt, aber wenigstens funktioniert nun alles auch ohne meine eher quick'n'dirty Lösung. Danke euch allen :thumb:

[Uwe Raabe]

Wäre es nicht geschickter, den aktuellen Progress-Wert einfach in deinem Thread zu setzen und im Hauptthread entweder über OnIdle, ActionUpdate oder einen Timer-Event zu aktualisieren. Gegebenfalls musst du die Zugriffe auf den Progress-Wert noch durch einen TMultiReadExclusiveWriteSynchronizer absichern, was aber mit einer passenden Getter/Setter-Kombination transparent zu realisieren wäre.

Der Vorteil bei diesem Verfahren ist, daß der Thread von der Synchronisierung entlastet wird und der Hauptthread nur dann was aktualisiert, wenn er dazu auch Zeit hat.

Abgesehen davon ist das IMHO auch viel übersichtlicher.

[Zacherl]

Für den Progress könnte ich es mit etwas Mühe so machen, das stimmt. Etwas Mühe deshalb, weil es sich um ein Protokoll handelt, welches simultane Transfer über ein einzelnes Socket realisiert (praktisch ein Multiplexer / Demultiplexer, wobei der Thread zum Multiplexer Teil gehört). Ich müsste also z.b. im Timer auch auf die ThreadList zugreifen, die alle Transfer Objekte enthält und dann alles iterieren. Somit würde beim Zugriff auf LockList auch wieder mit dem Thread synchronisiert.

Das eigentliche Problem sind aber eher die anderen Ereignisse, die man nicht so einfach ändern kann. Das Protokoll kann beispielsweise ganze Dateien verschicken. Dabei soll die Datei allerdings NICHT zuerst in den Speicher geladen und dann verschickt werden. Genauso gestaltet sich der Empfang. Es gibt ein OnTransferData() Event, welches von mir dafür gedacht wurde, die im aktuellen Paket empfangenen Daten in die entsprechende Datei zu schreiben.
Die Daten werden aus verschiedenen Gründen in 8 KiB Blöcke gestückelt. Zum einen natürlich, damit mehrere Übertragungen simultan laufen können und zum zweiten aufgrund einer "on the fly" Verschlüsselung und Kompression.

[implementation]

Nehmen wir mal einen typischen Beispieldurchlauf:
1. (Thread) Liste wird gesperrt.
2. (VCL) Liste soll abgerufen werden, ist aber noch gesperrt -> es wird gewartet, bis die Liste wieder frei ist.
3. (Thread) Synchronisiert Event-Aufruf mit VCL-Thread.
4. Der VCL-Thread kann den Aufruf aber noch gar nicht synchronisieren, weil noch auf das Unlocken der Liste gewartet wird.

Und da ham wir den Deadlock. Beide Threads warten.

Edit: Oder hab ich da irgendwas falsch verstanden?

[Zacherl]

Ich bin nicht sicher. Aber es wäre auf jeden Fall logisch:
1) Thread: sperrt die Liste
2) Thread: Ereigniss wird aufgerufen
3) VLC: sperrt die Liste (wartet auf Freigabe durch Thread)
4) Ereigniss im Thread-Context: synchronisiert mit der VCL

Die Frage ist, wie genau die Synchronisierung stattfindet. Wenn ich vorher von der VCL aus die Liste sperre, wartet der VCL Thread ja aud Freigabe durch den Thread. Synchronisiere ich jetzt im Ereigniss, wird der entsprechende Code ja im Context des VCL Threads ausgeführt.
Interessant ist allerdings jetzt zu wissen, wie das ganze von statten geht. Wird darauf gewartet, dass der VCL Thread irgendwie einen Context Switch durchführt, wäre das ganz klar der Grund für den Deadlock. Ist dies der Fall? :?:

Ich frage deshalb, weil ich mal eine Funktion geschrieben hatte, die auch Code im Context eines fremden Threads ausführen kann. Dabei habe ich allerdings mit SetThreadContext() den EIP manipuliert. In diesem Falle würde der synchronisierte Code ja unabhängig vom Warten auf LockList ausgeführt, wonach dann wieder zum LockList Offset gesprungen wird.

[Khabarakh]

Synchronisierung funktioniert über Window Messages. Ist der UI-Thread also blockiert, wird tatsächlich nichts abgearbeitet und du hast deinen Deadlock :) .
Es gibt übrigens eine Regel, beim Halten eines Locks nie Events zu feuern. Denn ein Lock soll ja eigentlich feinkörnig sein, was beim Aufrufen unbekannter Methoden schlecht sichergestellt werden kann ;) .

[Zacherl]

Ah okay diese Regel war mir bisher unbekannt :mrgreen: Also wäre die optimale Lösung wirklich sowas in der Art:

Delphi-Quellcode:

procedure TdxIDTPSendThread.Execute;
var
  I, X, Y: Integer;
  List: TList;
  Transfer: TdxIDTPTransfer;
begin
  while (not Terminated) do
  begin
    List := FWriter.TransferList.LockList;
    try
      X := 0;
      Y := 0;
      for I := 0 to List.Count - 1 do
      begin
        FWriter.TransferList.UnlockList;
        try
          // OnProgress
        finally
          List := FWriter.TransferList.LockList;
        end;

        // blabla ..  
      
        FWriter.TransferList.UnlockList;
        try
          // OnData
        finally
          List := FWriter.TransferList.LockList;
        end;

        if (not Transfer.Priority) then
        begin
          Inc(X);
        end;
        Inc(Y);
      end;
      // Beendete Transfers entfernen
      for I := List.Count - 1 downto 0 do
      begin
        Transfer := TdxIDTPTransfer(List[I]);
        if (Transfer.TransferState = tsFinished) then
        begin
          List.Delete(I);
          Transfer.Free;
        end;
      end;
    finally
      FWriter.TransferList.UnlockList;
    end;
    Sleep(10);
    // Thread suspendieren
    if (Y = 0) then
    begin
      {$WARNINGS OFF}
      Suspend;
      {$WARNINGS ON}
    end;
  end;
end;

Edit: Wobei dann hätte ich halt wieder das Problem, dass von der VCL aus Elemente in der Liste geändert oder sogar entfernt werden könnten. Ist der Thread dann noch in der Schleife und das letzte Element fehlt auf einmal, dann krachts ja wieder :(

[himitsu]

Wenn du die Möglichkeit hast, dann wäre es wohl besser die Events außerhalb der "Hauptsperrung" abzuarbeiten.

statt deinem

Code:

thread-schleife
begin
  sperren
    schleife
    begin
      entsperren
        event aufrufen
      sperren
      ...
    end;
  entsperren
  kleine pause
end;

nur einmal sperren und danach die Events abarbeiten

Code:

thread-schleife
begin
  sperren
    schleife
    begin
      event zu 'ner liste hinzufügen
      ...
    end;
  entsperren
  alle gespeicherten events aufrufen
  kleine pause
end;

oder

Code:

thread-schleife
begin
  sperren
    daten schnell aus liste auslesen
  entsperren
  schleife
  begin
    (mit den ausgelesenen/kopierten daten arbeiten)
    event abarbeiten
    ...
  end;
  kleine pause
end;

[Zacherl]

Möglichkeit 3 kommt leider nicht in Frage, aber Nummer 2 sieht interessant aus. Werde ich mal ausprobieren. Solange da von VCL Seiten aus keine Objekte gelöscht werden, müsste das gut gehen. Ansonsten verweisen die Ereignisse in der Liste wiederrum auf Datenobjekte die nicht mehr existieren ..

Edit: Wobei .. Möglichkeit 3 klingt sogar am besten :mrgreen: Die Objekte selbst "sollten" bei korrekter Verwendung der Komponente nur im Thread selbst freigegeben werden. Und wenn die Reihenfolge der Liste von der VCL aus geändert wird, dann würde sich das zwar erst im nächsten Zyklus des Threads bemerkbar machen, aber das wäre nicht weiter tragisch. Hinzufügen von Objekten ist ja ebenfalls okay.

[Zacherl]

Dann mal vielen Dank an alle die geholfen haben :thumb: Der DeadLock entstand tatsächlich wie weiter oben beschrieben. Behoben habe ich das Problem nun, indem ich himitsus Variante 3 verwendet habe.