6
\$\begingroup\$

I am refining my reliable UDP library, its at its 3rd iteration now. It is quiet a bit multi-threaded with almost all major operations having dedicated threads. I need your opinion on the following critical worker methods.

Legend

TAgUDP          : Indy UDP Server derived class.
TAgUDPPeer      : Class containing a peer's information.
    FPacketLostTimeout : 2 times the ping.
    SendIDPacket       : Constructs packets.

TAgList <T> = class ( TList <T> )
private

  fLock : TMREWSync;

public

  constructor Create;
  destructor  Destroy; override;

  procedure BeginRead; inline;
  procedure BeginWrite; inline;
  procedure EndRead; inline;
  procedure EndWrite; inline;

end;

TAgUDPOp        : Enumerator representing packet command types.

TAgUDPPacket = class
protected

  FID          : Word;
  FCommand     : TAgUDPOp;
  FSenderID    : TGUID;
  FRecipientID : TGUID;
  FMsg         : TAgBuffer;
  FTimeStamp   : TDateTime;
  FResendCount : Byte;

public

  constructor Create;
  destructor  Destroy; override;

end;

TAgUDPPeerState = ( AwaitingLoginReply, Connecting, OneWayP2P, Connected, Dead, SetForRemoval );

Packet Manager

procedure TAgUDP.PacketManager;

  procedure ManagePacketsForAllLists ( AList: TAgList <TAgUDPPeer> );
  var
    Peer    : TAgUDPPeer;
    Packet  : TAgUDPPacket;
    I       : NativeInt;
    J       : SmallInt;
    Diff    : Cardinal;
  begin

    AList.BeginRead;
    for I := AList.Count - 1 downto 0 do
    begin

      Peer := AList [I];
      if Peer.FState <> TAgUDPPeerState.Connected then continue;
      if Peer.SentPackets.Count = 0 then continue;

      for J := Peer.SentPackets.Count - 1 downto 0 do
      begin

        try

        Packet := Peer.SentPackets [J];
        if Packet.FResendCount < 5 then
        begin

          Diff := MillisecondsBetween ( Now, Packet.FTimeStamp );
          if ( Diff < Peer.FPacketLostTimeout ) and ( Diff <= 50 ) then continue;

          Packet.FTimeStamp   := Time;
          Packet.FResendCount := Packet.FResendCount + 1;

          if Peer.LocalPeer then
            SendBuffer ( Peer.FPrivateIP, Peer.FPrivatePort, TIdBytes ( Packet.FMsg.ToBytes ))
          else
            SendBuffer ( Peer.FPublicIP,  Peer.FPublicPort,  TIdBytes ( Packet.FMsg.ToBytes ));

        end
        else
          Peer.SentPackets.Delete ( J );

        except Peer.SentPackets.Delete ( J );
        end;

      end;

    end;
    AList.EndRead;

  end;

begin
  ManagePacketsForAllLists ( FClientsList );
  ManagePacketsForAllLists ( FServersList );
end;

Peer Manager

procedure TAgUDP.PeerManager;
var
  Peer            : TAgUDPPeer;
  I               : NativeInt;
  DT              : TDateTime;
  Diff            : NativeInt;
  ForceDisconnect : Boolean;
begin

  DT              := Now;
  ForceDisconnect := False;
  FClientsList.BeginWrite;
  for I := FClientsList.Count - 1 downto 0 do
  begin

    try

    Peer := FClientsList [I];
    if Peer.FState <> TAgUDPPeerState.SetForRemoval then
    begin

      Diff := MillisecondsBetween ( DT, Peer.FLastPacketTime );
      if Diff > FAliveTimeout then
      begin

        Peer.BeginWrite;
        Dec ( Diff, 30000 );
        if Diff > FRemoveTimeout then
        begin
          Peer.State     := TAgUDPPeerState.SetForRemoval;
          ForceDisconnect := True;
        end
        else
          Peer.State := TAgUDPPeerState.Dead;
        Peer.EndWrite;

      end;

    end
    else
    begin
      if FServerPersonality and ForceDisconnect then
        Peer.SendIDPacket ( nil, TAgUDPOp.ForcedEndOfSession );
      FClientsList.Delete ( I );
    end;

    except FClientsList.Delete ( I );
    end;

  end;
  FClientsList.EndWrite;

  if FServersList.Count = 0 then exit;
  FServersList.BeginWrite;
  for I := FServersList.Count - 1 downto 0 do
  begin

    try

    Peer := FServersList [I];

    if Peer.FState = TAgUDPPeerState.SetForRemoval then
    begin
      Peer.SendIDPacket ( nil, TAgUDPOp.EndOfSesion );
      FServersList.Delete ( I );
    end
    else
    begin

      Diff := MillisecondsBetween ( DT, Peer.FLastPacketTime );
      if Diff > fAliveTimeout then
      begin
        Peer.BeginWrite;
        Peer.State := TAgUDPPeerState.Dead;
        Peer.EndWrite;
      end;

    end;

    except FServersList.Delete ( I );
    end;

  end;
  FServersList.EndWrite;

end;

Please give some advice on improving them. Also will it be alright, if I merge these with each other i.e. they will be worked by a single thread. I was thinking of utilizing thread pools to accommodate groups of connected clients. The intended audience number in the thousands.

\$\endgroup\$
  • \$\begingroup\$ Please at least comment, not just up vote! \$\endgroup\$ – Umair Ahmed Mar 16 '14 at 7:47
  • \$\begingroup\$ I think there's very few Delphi gurus around here. It is easy to see that you've asked a good question, but most people probably don't know what to say :) I know some Delphi though, but it's been a while since I've used it. I'll try to take a look at your question later on. \$\endgroup\$ – Simon Forsberg Mar 18 '14 at 20:04
1
\$\begingroup\$

give some advice on improving them

Add some expository introductory comments describing how your library is to be used and called, what makes it "reliable", and how and why it's different from TCP (what most people think of for reliable UDP) and from RUDP. Explain when, where, and how you're dealing with timeouts, dropped packets, duplicate packets, corrupted packets, and all the other possible things that can go wrong.

Inside the code itself, add explanatory comments throughout describing why the code does what it does. For now, the code and lack of comments probably isn't seductive enough to attract much attention.

Also see this "reliable" UDP question/answer.

\$\endgroup\$
  • \$\begingroup\$ Noted, I'll do just that soon enough. \$\endgroup\$ – Umair Ahmed Apr 9 '14 at 5:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.