Thread and TCP

[ingalime]

Hello everyone.
In Windows we use:

Code: Select all

  IdTCPClient1->Connect();
     try
     {
      IdTCPClient1->IOHandler->Write("********");
     }
      __finally
        {
         IdTCPClient1->Disconnect();
        }


We read that for the mobile platform, this code should be allocated to a separate thread.
Please show an example.

[rlebeau]

We read that for the mobile platform, this code should be allocated to a separate thread.

Yes. Mobile OSes do not like having an app's main thread blocked for any length of time. The OS will just kill the app. Desktop platforms are more lenient about that.

Please show an example.

There are numerous examples floating around if you search online.

[ingalime]

So it will be correct?
Main:

Code: Select all

TFormSent *FormSent;
TMyConnect * MyConnect;
//---------------------------------------------------------------------------
__fastcall TFormSent::TFormSent(TComponent* Owner)
   : TForm(Owner)
{
}
//---------------------------------------------------------------------------
void __fastcall TFormSent::Button1Click(TObject *Sender)
{
 MyConnect = new MyConnect(false);
 MyConnect->FreeOnTerminate = true;
}


h TThread

Code: Select all

class TMyConnect : public TThread
{
private:
protected:
   void __fastcall Execute();
   void __fastcall TryMyConnect();
public:
   __fastcall TMyConnect(bool CreateSuspended);
};


cpp TThread

Code: Select all

__fastcall TMyConnect::TMyConnect(bool CreateSuspended)
   : TThread(CreateSuspended)
{
}
//---------------------------------------------------------------------------
void __fastcall TMyConnect::Execute()
{
   Synchronize(&TryMyConnect);
}
//---------------------------------------------------------------------------

void __fastcall TMyConnect::TryMyConnect()
{
 try
  {
   try{
      FormSent->IdTCPClient1->Connect();
      FormSent->IdTCPClient1->IOHandler->Write(FormSent->Memo1->Lines, true, IndyTextEncoding_UTF8());
     }
     catch(const Exception &E)
         {
         //
           }

  }
   __finally
  {
   FormSent->IdTCPClient1->Disconnect();
  }
}


[rlebeau]

So it will be correct?

No. You are synchronizing *all* of TMyConnect's work back to the main thread, where it doesn't belong. You are defeating the purpose of using a worker thread. You need to get rid of the Synchronize(&TryMyConnect) call and just run the code directly in Execute() so it runs in the context of the worker thread. Use Synchronize() only when you need to run code that must run in the main thread, like accessing the UI.

Try something more like this:

Main:

Code: Select all

TFormSent *FormSent;
TMyConnect * MyConnect = NULL;
//---------------------------------------------------------------------------
__fastcall TFormSent::TFormSent(TComponent* Owner)
   : TForm(Owner)
{
}
//---------------------------------------------------------------------------
void __fastcall TFormSent::Button1Click(TObject *Sender)
{
    if (!MyConnect)
    {
        MyConnect = new MyConnect();
        MyConnect->OnTerminate = &ThreadTerminated;
        MyConnect->Resume(); // or ->Start() in CB2010+
    }
}
//---------------------------------------------------------------------------
void __fastcall TFormSent::ThreadTerminated(TObject *Sender)
{
    MyConnect = NULL;
}


h TThread

Code: Select all

class TMyConnect : public TThread
{
private:
    TStringList *FLines;
    void __fastcall GetLines();
protected:
    void __fastcall Execute();
public:
    __fastcall TMyConnect();
};


cpp TThread

Code: Select all

__fastcall TMyConnect::TMyConnect()
   : TThread(true)
{
    FreeOnTerminate = true;
}
//---------------------------------------------------------------------------
void __fastcall TMyConnect::GetLines()
{
    FLines->Assign(FormSent->Memo1->Lines);
}
//---------------------------------------------------------------------------
void __fastcall TMyConnect::Execute()
{
    try
    {
        FormSent->IdTCPClient1->Connect();
        try
        {
            FLines = new TStringList;
            try
            {
                Synchronize(&GetLines);
                FormSent->IdTCPClient1->IOHandler->Write(FLines, true, IndyTextEncoding_UTF8());
            }
            __finally
            {
                delete FLines;
            }
        }
        __finally
        {
            FormSent->IdTCPClient1->Disconnect();
        }
    }
    catch(const Exception &E)
    {
        //
    }
}


Or this, which eliminates the need for using Synchronize() to get the data to send:

Main:

Code: Select all

TFormSent *FormSent;
TMyConnect * MyConnect = NULL;
//---------------------------------------------------------------------------
__fastcall TFormSent::TFormSent(TComponent* Owner)
   : TForm(Owner)
{
}
//---------------------------------------------------------------------------
void __fastcall TFormSent::Button1Click(TObject *Sender)
{
    if (!MyConnect)
    {
        MyConnect = new MyConnect(Memo1->Lines);
        MyConnect->OnTerminate = &ThreadTerminated;
        MyConnect->Resume(); // or ->Start()
    }
}
//---------------------------------------------------------------------------
void __fastcall TFormSent::ThreadTerminated(TObject *Sender)
{
    MyConnect = NULL;
}


h TThread

Code: Select all

class TMyConnect : public TThread
{
private:
    TStringList *FLines;
protected:
    void __fastcall Execute();
public:
    __fastcall TMyConnect(TStrings *ALines);
    __fastcall ~TMyConnect();
};


cpp TThread

Code: Select all

__fastcall TMyConnect::TMyConnect(TStrings *ALines)
   : TThread(true), FLines(new TStringList)
{
    FreeOnTerminate = true;
    FLines->Assign(ALines);
}
//---------------------------------------------------------------------------
__fastcall TMyConnect::~TMyConnect()
{
    delete FLines;
}
//---------------------------------------------------------------------------
void __fastcall TMyConnect::Execute()
{
    try
    {
        FormSent->IdTCPClient1->Connect();
        try
        {
            FormSent->IdTCPClient1->IOHandler->Write(FLines, true, IndyTextEncoding_UTF8());
        }
        __finally
        {
            FormSent->IdTCPClient1->Disconnect();
        }
    }
    catch(const Exception &E)
    {
        //
    }
}


Either way, I would suggest NOT placing the TIdTCPClient on the Form at all. Especially if you ever need to have multiple connections running in parallel. Move the TIdTCPClient into the thread instead, eg:

Main:

Code: Select all

TFormSent *FormSent;
TMyConnect * MyConnect = NULL;
//---------------------------------------------------------------------------
__fastcall TFormSent::TFormSent(TComponent* Owner)
   : TForm(Owner)
{
}
//---------------------------------------------------------------------------
void __fastcall TFormSent::Button1Click(TObject *Sender)
{
    MyConnect = new MyConnect("host", 12345, Memo1->Lines);
}


h TThread

Code: Select all

class TMyConnect : public TThread
{
private:
    String FHost;
    Word FPort;
    TStringList *FLines;
protected:
    void __fastcall Execute();
public:
    __fastcall TMyConnect(String AHost, Word APort, TStrings *ALines);
    __fastcall ~TMyConnect();
};


cpp TThread

Code: Select all

__fastcall TMyConnect::TMyConnect(String AHost, Word APort, TStrings *ALines)
   : TThread(false), FHost(AHost), FPort(APort), FLines(new TStringList)
{
    FreeOnTerminate = true;
    FLines->Assign(ALines);
}
//---------------------------------------------------------------------------
__fastcall TMyConnect::~TMyConnect()
{
    delete FLines;
}
//---------------------------------------------------------------------------
void __fastcall TMyConnect::Execute()
{
    try
    {
        TIdTCPClient *Client = new TIdTCPClient;
        try
        {
            Client->Host = FHost;
            Client->Port = FPort;

            Client->Connect();
            try
            {
                Client->IOHandler->Write(FLines, true, IndyTextEncoding_UTF8());
            }
            __finally
            {
                Client->Disconnect();
            }
        }
        __finally
        {
            delete Client;
        }
    }
    catch(const Exception &E)
    {
        //
    }
}


[ingalime]

Thank you rlebeau for your help.
Please check my code with ActivityIndicator1:

Code: Select all

void __fastcall TFormSent::Button1Click(TObject *Sender)
{
 //read host and port from ini file   
 // String FHost;
 // Word FPort; 
  Button1->Enabled = false; //The user does not have to press the button more than once
  ActivityIndicator1->Animate = true;   
  MyConnect = new MyConnect(FHost, FPort, Memo1->Lines);
}


Code: Select all

__fastcall TMyConnect::~TMyConnect()
{
    Button1->Enabled = true
    ActivityIndicator1->Animate = false;     
   delete FLines;
}


And what do you recommend to write in catch TMyConnect::Execute?

[rlebeau]

Please check my code with ActivityIndicator1:

Your worker thread is using FreeOnTerminate=true, so its destructor runs in the context of the worker thread (after Execute() exits), not in the context of the main UI thread. To update your UI when the thread is finished, use the thread's OnTerminate event, which is synchronized with the main UI thread (before the thread object is freed):

Code: Select all

void __fastcall TFormSent::Button1Click(TObject *Sender)
{
    Button1->Enabled = false;
    ActivityIndicator1->Animate = true;   
    MyConnect = new MyConnect(FHost, FPort, Memo1->Lines);
    MyConnect->OnTerminate = &ThreadTerminated;
    MyConnect->Resume(); // or ->Start()
}

void __fastcall TFormSent::ThreadTerminated(TObject *Sender)
{
    Button1->Enabled = true;
    ActivityIndicator1->Animate = false;     
}


Code: Select all

__fastcall TMyConnect::TMyConnect(String AHost, Word APort, TStrings *ALines)
    : TThread(true), FHost(AHost), FPort(APort)
{
    FLines = new TStringList;
    FLines>Assign(ALines);
}

__fastcall TMyConnect::~TMyConnect()
{
    delete FLines;
}


And what do you recommend to write in catch TMyConnect::Execute?

Write whatever you want. Preferably, you shouldn't have the catch at all. That way, an uncaught exception will terminate the thread and populate the thread's FatalException property, which you can check to know if it terminated gracefully or abortively:

Code: Select all

void __fastcall TFormSent::ThreadTerminated(TObject *Sender)
{
    Button1->Enabled = true;
    ActivityIndicator1->Animate = false;     

    if (static_cast<TMyConnect*>(Sender)->FatalException)
    {
        // something unexpected happened...
        // use FatalException as needed...
    }
}


[ingalime]

Thank you very much!

P.S.
I rty to use: IdTCPClient1->Connect();
without thread, in main thread. I use tablet with 2 Gb memory. I test one week. No problem.
Do I need realy separate thread for this device?

[rlebeau]

I rty to use: IdTCPClient1->Connect();
without thread, in main thread. I use tablet with 2 Gb memory. I test one week. No problem.
Do I need realy separate thread for this device?

DO NOT call ANY potentially-blocking function in the context of the main UI thread at all. That includes TIdTCPClient::Connect(). You MUST use it in a worker thread on a mobile OS, yes.

[ingalime]

Many thanks rlebeau. I'm using your code. Everything is working.
Summarizing:
Main:

Code: Select all

TFormMenu *FormMenu;
TMyConnect * MyConnect = NULL;
extern String ip;
extern int port;
//---------------------------------------------------------------------------
__fastcall TFormMenu::TFormMenu(TComponent* Owner)
   : TForm(Owner)
{
}
//---------------------------------------------------------------------------
void __fastcall TFormMenu::Button1Click(TObject *Sender)
{
  Close();
}
//---------------------------------------------------------------------------
void __fastcall TFormMenu::ThreadTerminated(TObject *Sender)
{
    Button1->Enabled = true;
    AniIndicator1->Visible = false;
}
//---------------------------------------------------------------------------
void __fastcall TFormMenu::Button2Click(TObject *Sender)
{

  if(Memo1->Lines->Count > 0)
   {
   Button1->Enabled = false;
   AniIndicator1->Visible = true;
   MyConnect = new TMyConnect(ip, port, Memo1->Lines);
        MyConnect->OnTerminate = &ThreadTerminated;
   MyConnect->Start();
   }

}


h TThread

Code: Select all

class TMyConnect : public TThread
{
   private:
      String FHost;
      Word FPort;
      TStringList *FLines;
   protected:
      void __fastcall Execute();
   public:
      __fastcall TMyConnect(String AHost, Word APort, TStrings *ALines);
      __fastcall ~TMyConnect();
};


cpp TThread

Code: Select all

#include <IdTCPClient.hpp>

//---------------------------------------------------------------------------
__fastcall TMyConnect::TMyConnect(String AHost, Word APort, TStrings *ALines)
    : TThread(true), FHost(AHost), FPort(APort)
{
    FLines = new TStringList;
    FLines->Assign(ALines);
}
//---------------------------------------------------------------------------
__fastcall TMyConnect::~TMyConnect()
{
   delete FLines;
}
//---------------------------------------------------------------------------
void __fastcall TMyConnect::Execute()
{
    try
    {
      TIdTCPClient *Client = new TIdTCPClient;
        try
        {
            Client->Host = FHost;
            Client->Port = FPort;
            Client->ConnectTimeout = 5000;
            Client->Connect();
            try
            {
                Client->IOHandler->Write(FLines, true, IndyTextEncoding_UTF8());
            }
            __finally
            {
                Client->Disconnect();
            }
        }
        __finally
        {
            delete Client;
        }
    }
    catch(const Exception &E)
    {
    String er = L"Sorry, there is no connection to the server.";
    //???
    }
}


I still have a question about how to correctly display a message with a string:
String er = L"Sorry, there is no connection to the server.";

I can't use ShowMessage because in thread is not safe.
I can't use MessageBoxW:
::MessageBoxW(NULL, er.c_str(), Application->Title.c_str(), MB_OK | MB_ICONERROR);
Undefined symbol 'Application'

I try like this:

Code: Select all

void __fastcall TFormMenu::ThreadTerminated(TObject *Sender)
{
    Button1->Enabled = true;
   AniIndicator1->Visible = false;
    if (static_cast<TMyConnect*>(Sender)->FatalException)
    {
    String er = L"Sorry, there is no connection to the server.";
    ShowMessage(er);
   }

}


But it does not work.

What are your recommendations? How correctly to show the message to the user?
Thanks.

[ingalime]

I think so:
Main h

Code: Select all

__published:   
   void __fastcall SyncShowMessage();
private:
//***


Main cpp

Code: Select all

int chek = 0; //global

void __fastcall TFormMenu::SyncShowMessage()
{
 AniIndicator1->Visible = false;
 chek = 1;
 ShowMessage(L"Sorry, there is no connection to the server.");
}

void __fastcall TFormMenu::ThreadTerminated(TObject *Sender)
{
    Button1->Enabled = true;
    AniIndicator1->Visible = false;
    Close();//close Form
}

void __fastcall TFormMenu::FormShow(TObject *Sender)
{
 chek = 0;
}
//---------------------------------------------------------------------------

void __fastcall TFormMenu::FormClose(TObject *Sender, TCloseAction &Action)
{
 if(chek == 0)
   ShowMessage(L"Thank you! All good.");
}


cpp Thread

Code: Select all

//******
 __finally
        {
            delete Client;
        }
    }
    catch(const Exception &E)
    {
         
    Synchronize(&FormMenu->SyncShowMessage);
   }


[rlebeau]

Code: Select all

MyConnect = new TMyConnect(ip, port, Memo1->Lines);
MyConnect->OnTerminate = &ThreadTerminated;
MyConnect->Start();


You are not using FreeOnTerminate=true anymore, so you are leaking the thread object when it is done running.

I still have a question about how to correctly display a message with a string:
String er = L"Sorry, there is no connection to the server.";

I can't use ShowMessage because in thread is not safe.
I can't use MessageBoxW:
::MessageBoxW(NULL, er.c_str(), Application->Title.c_str(), MB_OK | MB_ICONERROR);
Undefined symbol 'Application'

The global Application object is defined in the Vcl.Forms.hpp and Fmx.Forms.hpp header files.

I try like this:

Code: Select all

void __fastcall TFormMenu::ThreadTerminated(TObject *Sender)
{
    Button1->Enabled = true;
   AniIndicator1->Visible = false;
    if (static_cast<TMyConnect*>(Sender)->FatalException)
    {
    String er = L"Sorry, there is no connection to the server.";
    ShowMessage(er);
   }

}


But it does not work.

That is because your thread's Execute() method is catching and discarding all possible exceptions, so the FatalException property is never set. You need to get rid of the try/catch in your Execute() method. Or, catch exceptions but then throw your own exception with the desired Message assigned to it. Either way, you have to let an exception escape Execute() in order to use the FatalException property.

[rlebeau]

I think so:
...

You are basically just duplicating what OnTerminate+FatalException can already do. OnTerminate is synchronized with the main thread, and FatalException is set if Execute() exits due to an uncaught exception.

[ingalime]

You are not using FreeOnTerminate=true anymore, so you are leaking the thread object when it is done running.

Many thanks for this remark. Now so:

Code: Select all

__fastcall TMyConnect::TMyConnect(String AHost, Word APort, TStrings *ALines)
    : TThread(true), FHost(AHost), FPort(APort)
{
   FreeOnTerminate = true;
   FLines = new TStringList;
   FLines->Assign(ALines);
}


[ingalime]

On Windows everything works as expected. In Android NO.
In Android everything works as expected if server ON. If server OFF:
segmentation fault 11 See all debug windows on picture.
And catch does not work. Application crash if server OFF.

Code: Select all

void __fastcall TMyConnect::Execute()
{
   try
   {
      TIdTCPClient *Client = new TIdTCPClient;
      try
      {
         Client->Host = FHost;
         Client->Port = FPort;
         Client->ConnectTimeout = 5000;

         Client->Connect(); //here all debug windows and then crash application if server OFF
         try
         {
            Client->IOHandler->Write(FLines, true, IndyTextEncoding_UTF8());
         }
         __finally
         {
            Client->Disconnect();
         }
      }
      __finally
      {
            delete Client;
      }
   }
   catch(...)//try catch all
   {
    //try catch all but breakpoint does not work
    Synchronize(&FormMenu->SyncShowMessage);//breakpoint does not work
   }
}


When the server OFF may need to add something to the properties TIdTCPClient if platform Android?

[rlebeau]

On Windows everything works as expected. In Android NO.
In Android everything works as expected if server ON. If server OFF:
segmentation fault 11 See all debug windows on picture.

I know for a fact that Indy works on Android. You are just going to have to debug the code to find the root culprit.

And catch does not work.

You shouldn't be using catch(...) to begin with. Even on Windows, it is not guaranteed to handle Delphi-style exceptions correctly. Catch exceptions by type explicitly, eg: catch(const Exception&).

Even better, just get rid of the try/catch altogether, and let the exception terminate the thread and set its FatalException property.

Also, you can get rid of the try/__finally by using a smart pointer like std::unique_ptr.

Code: Select all

#include <memory>

void __fastcall TMyConnect::Execute()
{
   std::unique_ptr<TIdTCPClient> Client(new TIdTCPClient);

   Client->Host = FHost;
   Client->Port = FPort;
   Client->ConnectTimeout = 5000;

   Client->Connect();
   Client->IOHandler->Write(FLines, true, IndyTextEncoding_UTF8());
   Client->Disconnect();
}

...

MyConnect = new TMyConnect(...);
MyConnect->OnTerminate = &ThreadTerminated;
MyConnect->Start();

...

void __fastcall TFormMenu::ThreadTerminated(TObject *Sender)
{
   TMyConnect *Thread = static_cast<TMyConnect>(Sender);
   ...
   if (Thread->FatalException)
      ShowMessage(static_cast<Exception*>(Thread->FatalException)->Message);
   ...
}


When the server OFF may need to add something to the properties TIdTCPClient if platform Android?

No, you do not.

The two exceptions you showed are perfectly normal. When the ConnectTimeout property is not 0 (the default), Connect() uses a worker thread to perform the actual connection. The thread terminates when the connection is established or an error occurs. Connect() waits up to the ConnectTimeout for the thread to terminate. If it does not terminate in time, Connect() closes the socket and terminates the thread.

The EIdSocketError exception you see is the actual connection failing. The EIdConnectTimeout exception you see is raised only if the thread did not terminate before the ConnectTimeout elapsed. Indy catches the EIdSocketError internally (which probably happens when Indy closes the socket, or just before), and EIdConnectTimeout is raised into your code for further handling as needed.

The debugger sees all exceptions before your code does. That is normal behavior. You can tell the debugger to ignore these Indy exceptions, or all Indy exceptions by ignoring EIdException.

These exceptions should not be causing a segfault, though. So something else is going on. You are going to have to debug further to find it.