Page 1 of 2

Thread and TCP

PostPosted: Thu Nov 09, 2017 6:52 am
by 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.

Re: Thread and TCP

PostPosted: Thu Nov 09, 2017 12:51 pm
by rlebeau
ingalime wrote: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.

ingalime wrote:Please show an example.


There are numerous examples floating around if you search online.

Re: Thread and TCP

PostPosted: Fri Nov 17, 2017 2:16 am
by 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();
  }
}

Re: Thread and TCP

PostPosted: Fri Nov 17, 2017 10:58 am
by rlebeau
ingalime wrote: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)
    {
        //
    }
}

Re: Thread and TCP

PostPosted: Sat Nov 18, 2017 9:28 am
by 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?

Re: Thread and TCP

PostPosted: Mon Nov 20, 2017 11:38 am
by rlebeau
ingalime wrote: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;
}


ingalime wrote: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...
    }
}

Re: Thread and TCP

PostPosted: Thu Nov 23, 2017 11:50 am
by 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?

Re: Thread and TCP

PostPosted: Mon Nov 27, 2017 9:30 pm
by rlebeau
ingalime wrote: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.

Re: Thread and TCP

PostPosted: Tue Nov 28, 2017 7:16 am
by 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.

Re: Thread and TCP

PostPosted: Tue Nov 28, 2017 7:39 am
by 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);
   }

Re: Thread and TCP

PostPosted: Tue Nov 28, 2017 12:20 pm
by rlebeau
ingalime wrote:
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.

ingalime wrote: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 adnd Fmx.Forms.hpp header files.

ingalime wrote: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.

Re: Thread and TCP

PostPosted: Tue Nov 28, 2017 12:26 pm
by rlebeau
ingalime wrote: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.

Re: Thread and TCP

PostPosted: Wed Nov 29, 2017 2:25 am
by 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);
}

Re: Thread and TCP

PostPosted: Wed Nov 29, 2017 3:13 am
by 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?

Re: Thread and TCP

PostPosted: Wed Nov 29, 2017 12:58 pm
by rlebeau
ingalime wrote: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.

ingalime wrote: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);
   ...
}


ingalime wrote: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.