About Indy 10 TIdTCPServer and ramdom disconections

[PabloRomero]

Dear Indy People.

I have a serious problem and I can't find any information.

I have a service made with Delphi and Indy 10. I made it with TIdTCPServer.

It's a service that controls licenses with a "keep alive". Everything is based using the TIdTCPClient that runs the applications, exchanging information with the server service.

They always exchange XML based structures, like text. In this XML, there is a "command" property. This property receives every 15 minutes the command
"Keep" and the server answers "ok, here I am, Keep ok" and gives to the station that runs my application, status and login information.

All the information is stored in memory in a structure based on Tlist<Types>.

We know that there are threads that are executed in the TidTCPExecute event for each connection.

In my services there are another parallel threads inside the service, to maintain it, renew licenses and clean up unused licenses.

Everything accesses structures based on that Tlist<Lot of Types>. There are of course, other data sharewd by all those threads.

Everything works ok.

But...

Here comes the problem. My application disconnects randomly, to some workstations. I suspect it must be because of the way I protect the structures and data shared by the threads.

Going in detail

We have the IdTCPServerExecute event

Code:

var
    ClientIPAddr: string;

begin
      if not Server.Active then
        exit;

      ClientIPAddr := AContext.Binding.PeerIP;    //ip
      xComando.Linea := fxLeerLinea; //read the line
      if xComando.Linea = '' then 
      begin
        LimpiamosBuffer(AContext); //clean up buffer
        exit;
      end;

      xComando.EsXML := fxEsXML( xComando.Linea ); //is an xml what is coming

      if xComando.EsXML then
        TManejoPaquetesDOM.Procesa( xComando.Linea, Acontext, Server ) //process de estructure and comand. See below
      else
      begin
        TumbarIP(ClientIPAddr);    //clean the connection
        exit;
      end;
      xComando.Linea := ''; //clean line
end;

The "TManejoPaquetesDOM.Procesa( xComando.Linea, Acontext, Server )" is a class like this

Code:

  TManejoPaquetesDOM = class(TIdSync)
  protected
    fLinea: string;
    FContexto: TIdContext;
    FServer  : TIdTCPServer;
    FDespacho: IXMLPaqueteType;

    function  TomaObjXML( linea: string ): IXMLPaqueteType;
    function  fxComandoSoportado( const sComando: string ): boolean;
    procedure EnviaXML;
    procedure AltaEstacion;
    procedure EnvioModulos;
    procedure KeepAlive;
    procedure Informe;
    procedure LLSNoDisponible( pMantenimiento: boolean = false );
    procedure LoginEstacion;
    procedure LimpiaPaquete;
    procedure prExitSesion( pAccion: TManejoSesion; msg : string = '' );
    procedure DoSynchronize; override;

  public
    constructor Crear(linea: string; AContext: TIdContext; pServer: TidTCPServer );
    class procedure Procesa(Linea: string; AContext: TIdContext; pServer: TidTCPServer );
  end;

The "DoSynchronize" does all the work with each connection receive from clients.

Code:

class procedure TManejoPaquetesDOM.Procesa(Linea: string; AContext: TIdContext; pServer: TidTCPServer );
// with this class procedure we manage the thread
begin
    with Crear(linea, AContext, pServer ) do
    try
      Synchronize;
    finally
      Free;
    end;
end;

//the constructor creates, passes the data and parses the XML that brings each station
constructor TManejoPaquetesDOM.Crear(linea: string; AContext: TIdContext; pServer: TIdTCPServer );
begin
    inherited Create;
    fLinea    := linea;

    FContexto  := AContext;
    FContexto.Connection.IOHandler.DefStringEncoding := IndyTextEncoding_UTF8;

    FServer    := pServer;
    FDespacho  := TomaObjXML( linea );
end;

As you can see, "TManejoPaquetesDOM = class(TIdSync)". This is who processes everything. As I understand it, TidSync makes all the other threads
of IdTCPServerExecute threads wait for it to finish.

I'm not sure if this is the best approach, ideally the process should be parallel without making anyone wait.

All the structures when accessed by the other threads of my application are protected with a generic class I made
like this

Code:

  TControlBloqueos = class
  private
    FLlave: TCriticalSection;
  public

    constructor Create;
    destructor Destroy; override;

    procedure Lock;
    procedure Unlock;
  end;

implementation

constructor TControlBloqueos.Create;
begin
  inherited;
  FLlave := TCriticalSection.Create;
end;

destructor TControlBloqueos.Destroy;
begin
  Fllave.Free;
  inherited;
end;

procedure TControlBloqueos.Unlock;
begin
  Fllave.Leave;
end;

procedure TControlBloqueos.Lock;
begin
  Fllave.Enter;
end;

Each time the data shared by threads are accessed, those threads protect the data with an instance of this class, global to the whole service.

This instance is created at the start of the service

Code:

Bloqueo :=  TControlBloqueos.Create;

and all the threads accessing the data, proceed like this.

Code:

Bloqueo.lock;
try
    //process shared data between threads
finally
    Bloqueo.unlock;
end;

My problem is that I can't detect those random disconnections of some users from my system.

My questions

a) Is there a way to speed up what IdTCPServerExecute does? Not using TidSync?

b) What is the best way to protect shared data between threads? I'm not sure what I did is the best approach.

Regards
Pablo Romero

[rlebeau]

They always exchange XML based structures, like text. In this XML, there is a "command" property. This property receives every 15 minutes the command
"Keep" and the server answers "ok, here I am, Keep ok" and gives to the station that runs my application, status and login information.

15 minutes is a long time to keep a TCP connection sitting idle without any traffic. Some firewalls/routers will close an idle connection after just a few minutes. You might consider closing any TCP connection that is going to be sitting idle for awhile. Otherwise, assuming your "Keep" command is meant for keeping a license alive rather than keeping the TCP connection alive, you might consider sending a separate status/heartbeat command more frequently. Or even enabling TCP-level keep-alives on the connection itself.

All the information is stored in memory in a structure based on Tlist<Types>.

Since you are worried about synchronization, why not use TThreadList<Types> instead? Or at least put a TCriticalSection around your TList (which is what TThreadList does)?

Here comes the problem. My application disconnects randomly, to some workstations. I suspect it must be because of the way I protect the structures and data shared by the threads.

Possibly. TIdTCPServer does not disconnect clients randomly, however if a runtime Exception occurred while accessing your data, that could certainly kill your client threads.

As you can see, "TManejoPaquetesDOM = class(TIdSync)". This is who processes everything. As I understand it, TidSync makes all the other threads of IdTCPServerExecute threads wait for it to finish.

Not exactly. It makes a request to the main UI thread to run the specified object method, blocking the calling thread until that request is finished. So, other calls to TThread.Synchronize()/TIdThread.Synchronize() (and TIdNotify.Notify()/TThread.Queue()) will synchronize with each other. Other threads that are not performing sync requests will be unaffected.

I'm not sure if this is the best approach, ideally the process should be parallel without making anyone wait.

Then don't synchronize your processing. Only synchronize access to shared resources, like your TList, but do the actual processing outside of the syncs.

All the structures when accessed by the other threads of my application are protected with a generic class I made
like this
...
Each time the data shared by threads are accessed, those threads protect the data with an instance of this class, global to the whole service.

Then why are you synchronizing your processing if it is providing its own thread safety?

My problem is that I can't detect those random disconnections of some users from my system.

Are you running your code inside the debugger when the disconnects occur? Are you logging errors (especially in the TIdTCPServer.OnException event)?

a) Is there a way to speed up what IdTCPServerExecute does? Not using TidSync?

There is not enough context to answer that. We don't know what you are syncing and why, or what your processing is actually doing that might be getting bottlenecked.

b) What is the best way to protect shared data between threads? I'm not sure what I did is the best approach.

Again, not enough context to answer that, since we can't see what you are trying to protect exactly.

[PabloRomero]

Wow, Remy, I have lots of questions, but I'll be brief.

I think I was wrong about TidSync. I thought that a class(TidSync) runs itself, putting the rest of the connections on hold, until it finishes its job.

In fact -correct me if I am wrong- TidTCPServer.OnExecute executes every connection, that the server receives. So this block of code.

TidTCPServer.OnExecute

Code:

begin
      if not Server.Active then
        exit;

      ClientIPAddr := AContext.Binding.PeerIP;    //ip
      xComando.Linea := fxLeerLinea; //read the line
      if xComando.Linea = '' then
      begin
        LimpiamosBuffer(AContext); //clean up buffer
        exit;
      end;

      xComando.EsXML := fxEsXML( xComando.Linea ); //is an xml what is coming?

      if xComando.EsXML then
        TManejoPaquetesDOM.Procesa( xComando.Linea, Acontext, Server ) //process all my structure for taht connection
      else
      begin
        TumbarIP(ClientIPAddr);    //clean the connection
        exit;
      end;
      xComando.Linea := ''; //clean line
end;

So, the line

TMManagePackagesDOM.Process( xCommand.Line, Acontext, Server )

is executed for each connection that sends something to the server.

I thought "TManagePackagesDOM.Process(..)", because it's TidSync, it executes and put everything that entered OnExecute on hold, until it's finished. That's why I synchronize it.

So,

A) whether I synchronize or not, Do I have to protect all the data structures accessed by all my threads and everything that is touched inside TMManagePackagesDOM.Process(..) ?

I ask this because of what you say

"Then don't synchronize your processing. Only synchronize access to shared resources, like your TList, but do the actual processing outside of the syncs." (...)

and

"Then why are you synchronizing your processing if it is providing its own thread safety?"

B)
I am interested in this "Keep Alive" topic. I use it to keep licenses alive. But this point you say

"..Or even enabling TCP-level keep-alives on the connection itself."

Do dou have some example about this?

Regards, and thank you very much, my friend.

Pablo Romero
Cordoba - Argentina