Konzept: Netzwerkprotokoll

[BUG]

Wenn das Protokoll erstmal feststeht, kann man sicher noch beliebig optimieren.
Denkbar fände ich zum Beispiel eine mehrstufige Abbildung der IDs. Damit sparst du auch im angesprochenen Extremfall noch Speicher :)
Interessant könnte auch das Minimieren von Kopieroperationen (ganz abgesehen von Speicherallokation/-freigabe) innerhalb der Implementation sein (=> Allgemein, habe mir die Implementation noch nicht angeguckt).

[Sir Rufo]

Zu deinem Thread.Execute:

IMHO machst du da zuviel Gedöns in dem Thread, was da nicht hingehört.

Der IOHandler hat alle Informationen, wie was wo gesendet werden soll, allerdings nicht direkt wann, das kommt ja im Thread.

Also würde ich den Thread auf seine wesentlichen Sachen reduzieren und dann sieht das wie folgt aus

Delphi-Quellcode:

procedure TdxIDTPSendThread.Execute;
begin
  while not Terminated do
    begin
      WaitForSingleObject( FWaitEvent, INFINITE );
      if not Terminated then
        FIOHandler.SendDataPacket;
    end;
end;

Allerdings würde ich den IOHandler nicht an den Thread hängen, sondern den Thread an den IOHandler. Dann gibt es auch kein Gerangel wer den Thread beenden kann (wird ja auch über das FWaitEvent gesteuert).
Es könnte passieren, dass trotz Terminate der Thread nicht aufhört, weil direkt nach dem Terminate der Thread selber das FWaitEvent zurücksetzt ;)

[Zacherl]

Schonmal vielen Dank für eure Antworten!

Zitat:

Zitat von Sir Rufo (Beitrag 1184157)

Der IOHandler hat alle Informationen, wie was wo gesendet werden soll, allerdings nicht direkt wann, das kommt ja im Thread.

Diese Informationen sollten eigentlich alle im Thread sein. Der IOHandler stellt nach außen hin zwar auch eine Liste mit den ein- und ausgehenden Transfers zur Verfügung (FITransferList und FOTransferList), aber die sind eigentlich nur zum Ändern der Reihenfolge durch den Benutzer gedacht (bei ausgehenden Übertragungen). Im Grunde genommen ist der IOHandler nur ein Wrapper für den IOThread, weil ich nicht wollte, dass man von außen auf die threadspezifischen Methoden wie z.b. Suspend oder Terminate zugreifen kann. Du hast allerdings Recht, dass meine Logik da inkonsistent ist. Ich müsste die besagten Listen entweder auch in die Thread Klasse verschieben oder wirklich alles aus dem Thread in die IOHandler Klasse.

Zitat:

Zitat von Sir Rufo (Beitrag 1184157)

Allerdings würde ich den IOHandler nicht an den Thread hängen, sondern den Thread an den IOHandler. Dann gibt es auch kein Gerangel wer den Thread beenden kann (wird ja auch über das FWaitEvent gesteuert).
Es könnte passieren, dass trotz Terminate der Thread nicht aufhört, weil direkt nach dem Terminate der Thread selber das FWaitEvent zurücksetzt ;)

Der Thread wird vom IOHandler erzeugt und freigegeben. Die Terminate Methode des Threads habe ich so überschrieben, dass dort zusätzlich das FWaitEvent gesetzt wird. Also die Terminierung des Threads sollte soweit funktionieren (hat sie auch in meinen Tests :D). Die Instanz des IOHandlers übergebe ich dem Thread momentan noch, damit er auf die FOTransferList im IOHandler zugreifen kann.

Wer sich die aktuelle Implementierung mal als Ganzes anschauen möchte:
http://www.delphipraxis.net/170565-n...-new-post.html

[Sir Rufo]

Zitat:

Zitat von Zacherl (Beitrag 1184220)

Also die Terminierung des Threads sollte soweit funktionieren (hat sie auch in meinen Tests :D).

Ich habe auch nicht gesagt, dass das nicht funktioniert, es gibt aber ein ganz schmales Zeitfenster, wo das nicht funktioniert.
Das doofe daran ist, wenn man es gar nicht gebrauchen kann, dann schlägt das zu (Murphy) ;)

Hier mal mein Vorschlag für den Händler:

Delphi-Quellcode:

TIOHandler = class
private
  FSendQueue, FWaitQueue : TQueue<TdxIDTPOTransfer>;
  ...
protected
  ...
  procedure SendDataPacket;
  ...
end;

TIOHandler.SendDataPacket;
var
  Transfer : TdxIDTPOTransfer;
begin
  { CRITICAL-SECTION START }

  // Aus der Queue entnehmen
  Transfer := FSendQueue.Dequeue;

  { CRITICAL-SECTION ENDE }

  // Senden
  Transfer.SendNextPacketData;

  // Diesen Teil würde ich auch in eine eigene Methode packen, da diese auch aufgerufen werden sollte, 
  // wenn eine neue Transfer-Instanz hinzukommt

  { CRITICAL-SECTION START }

  // Mal schauen, was mit der Instanz weiter passieren soll
  if 
    ( Transfer.TransferState = tsActive {?} ) 
  and 
    ( Transfer.Priority or ( FSendQueue.Count < OutgoingTransferCountLimit ) )
  then
    FSendQueue.Enqueue( Transfer )
  else
    if 
      ( Transfer.TransferState <> tsFinished ) 
    then
      FWaitQueue.Enqueue( Transfer )
    else
      Transfer.Free;

  while ( FSendQueue.Count < OutgoingTransferCountLimit ) and ( FWaitQueue.Count > 0 ) do
    FSendQueue.Enqueue( FWaitQueue.Dequeue );

  if ( FSendQueue.Count = 0 ) then // hier noch die Abfrage rein, ob der Thread nicht eigentlich beendet werden soll
    ResetEvent( SendThread.WaitEvent )
  else
    SetEvent( SendThread.WaitEvent );

  { CRITICAL-SECTION ENDE }

end;

[Zacherl]

Zitat:

Zitat von Sir Rufo (Beitrag 1184246)

Ich habe auch nicht gesagt, dass das nicht funktioniert, es gibt aber ein ganz schmales Zeitfenster, wo das nicht funktioniert.
Das doofe daran ist, wenn man es gar nicht gebrauchen kann, dann schlägt das zu (Murphy) ;)

Irgendwie stehe ich hier auf dem Schlauch :D Ich prüfe doch direkt nach dem WaitForSingleObject auf Terminated. Und in der überschriebenen Terminate Funktion des Threads setze ich erst Terminated und dann löse ich das Event aus. Aber wie auch immer ..

Zitat:

Zitat von Sir Rufo (Beitrag 1184246)

Hier mal mein Vorschlag für den Händler

.. dein Ansatz mit der Queue sieht sehr gut aus :) Muss ich mir mal genauer anschauen, ob die TQueue Klasse von Delphi das Verschieben von Einträgen unterstüzt oder mir was eigenes basteln. Wollte dem User die Möglichkeit lassen, die Reihenfolge der Übertragungen zu ändern.

[Sir Rufo]

Zitat:

Zitat von Zacherl (Beitrag 1184259)

Zitat:

Zitat von Sir Rufo (Beitrag 1184246)

Ich habe auch nicht gesagt, dass das nicht funktioniert, es gibt aber ein ganz schmales Zeitfenster, wo das nicht funktioniert.
Das doofe daran ist, wenn man es gar nicht gebrauchen kann, dann schlägt das zu (Murphy) ;)

Irgendwie stehe ich hier auf dem Schlauch :D Ich prüfe doch direkt nach dem WaitForSingleObject auf Terminated. Und in der überschriebenen Terminate Funktion des Threads setze ich erst Terminated und dann löse ich das Event aus. Aber wie auch immer ..

Folgendes Szenario:
Der Thread holt sich über die LockList die Transfer-Elemente und kopiert diese. Dann gibt er diese LockList wieder frei. Genau zu diesem Zeitpunkt wird der Thread terminiert (FWaitEvent wird gesetzt und der Thread auf Terminated). Jetzt sind aber keine Elemente in der Liste, also setzt der Thread das FWaitEvent zurück und wartet darauf, dass das Event kommt (unendlich).
Der Thread wird also nicht beendet ;)

Dies ist zwar theoretisch aber ich habe bei genau sowas schon die kotzenden Pferde gesehen.
Bis du dann dem Entwickler dediziert beschreiben und vor allem nachweisen kannst, dass da was falsch läuft - gute Nacht Marie :evil:

Zitat:

Zitat von Zacherl (Beitrag 1184259)

Zitat:

Zitat von Sir Rufo (Beitrag 1184246)

Hier mal mein Vorschlag für den Händler

.. dein Ansatz mit der Queue sieht sehr gut aus :) Muss ich mir mal genauer anschauen, ob die TQueue Klasse von Delphi das Verschieben von Einträgen unterstüzt oder mir was eigenes basteln. Wollte dem User die Möglichkeit lassen, die Reihenfolge der Übertragungen zu ändern.

Dafür benötigst du eine sortierte Liste (mit einem Comparer)
TComparer<T>

Die Send-Queue würde ich aber genau so nehmen und die Reihenfolge nur für die wartenden Transfers berücksichtigen.

[Zacherl]

Zitat:

Zitat von Sir Rufo (Beitrag 1184261)

Dies ist zwar theoretisch aber ich habe bei genau sowas schon die kotzenden Pferde gesehen.
Bis du dann dem Entwickler dediziert beschreiben und vor allem nachweisen kannst, dass da was falsch läuft - gute Nacht Marie :evil:

Ahh danke für die Erläuterung, jetzt sehe ich das Problem auch. Habe eben schon ein DeadLock gefixt, da kann ich dir nur zustimmen: Bist man sowas zuverlässig reproduziert und gefunden hat, kann schonmal einige Zeit vergehen.

Den Comparer werde ich mir mal anschauen. Muss zugeben, dass ich mit den Generic Klassen von Delphi bisher kaum gearbeitet habe. Das Comperator Konzept ist mir allerdings von Java her bekannt. Prinzipiell könnte ich für die "WaitQueue" ja aber auch meine alte Lister weiter verwenden.

Eine Frage noch zu deinem Ansatz:
Du deklarierst die SendDataPacket Funktion als protected im IOHandler. Die Funktion muss allerdings vom Thread heraus aufgerufen werden. Ich weiß zwar, dass ein Zugriff (sogar auf private) Felder innerhalb der selben Unit klassenübergreifend möglich ist, aber meiner Meinung nach ist das eher schlechter Stil oder nicht?

[Sir Rufo]

Zitat:

Zitat von Zacherl (Beitrag 1184263)

Du deklarierst die SendDataPacket Funktion als protected im IOHandler. Die Funktion muss allerdings vom Thread heraus aufgerufen werden. Ich weiß zwar, dass ein Zugriff (sogar auf private) Felder innerhalb der selben Unit klassenübergreifend möglich ist, aber meiner Meinung nach ist das eher schlechter Stil oder nicht?

Nein, das ist kein schlechter Stil.

Die Methode SendDataPacket soll vom Thread aufgerufen werden können, aber nicht vom Rest der Welt.
Bei einigen Programmiersprachen habe ich diese Möglichkeit nicht und muss diese Methode tatsächlich als public deklarieren, obwohl ich das gar nicht möchte.

Delphi bietet mir die Möglichkeit (protected vs. strict protected) warum sollte man das dann nicht nutzen? :)

[Zacherl]

Zitat:

Zitat von Sir Rufo (Beitrag 1184268)

Die Methode SendDataPacket soll vom Thread aufgerufen werden können, aber nicht vom Rest der Welt.
Bei einigen Programmiersprachen habe ich diese Möglichkeit nicht und muss diese Methode tatsächlich als public deklarieren, obwohl ich das gar nicht möchte.

Delphi bietet mir die Möglichkeit (protected vs. strict protected) warum sollte man das dann nicht nutzen? :)

Weiß auch nicht, warum ich bisher immer davon ausgegangen bin, dass das schlechter Stil ist :) Das hilft mir auf jeden Fall sogar noch bei 1-2 weiteren OO Design Problemen im Netzwerkprotokoll weiter. Dann werde ich wohl auch hingehen und sämtliche Sende- und Empfangsmethoden vom Thread in den IOHandler verschieben, wo sie ja auch im Grunde genommen hingehören.

[Sir Rufo]

Zitat:

Zitat von Zacherl (Beitrag 1184270)

Zitat:

Zitat von Sir Rufo (Beitrag 1184268)

Die Methode SendDataPacket soll vom Thread aufgerufen werden können, aber nicht vom Rest der Welt.
Bei einigen Programmiersprachen habe ich diese Möglichkeit nicht und muss diese Methode tatsächlich als public deklarieren, obwohl ich das gar nicht möchte.

Delphi bietet mir die Möglichkeit (protected vs. strict protected) warum sollte man das dann nicht nutzen? :)

Weiß auch nicht, warum ich bisher immer davon ausgegangen bin, dass das schlechter Stil ist :) Das hilft mir auf jeden Fall sogar noch bei 1-2 weiteren OO Design Problemen im Netzwerkprotokoll weiter. Dann werde ich wohl auch hingehen und sämtliche Sende- und Empfangsmethoden vom Thread in den IOHandler verschieben, wo sie ja auch im Grunde genommen hingehören.

Weil es unorthodox aussieht und hier ja auch schon Diskussionen geführt worden sind, dass Delphi da inkonsequent wäre (im Vergleich zu anderen Sprachen). ;)