Following this Delphi sample and this other one, I built three scenarios in order to run properly TAniIndicator
along with WaitFor()
in the main thread.
In the first two tests, I use the Synchronize()
and Queue()
methods with FreeOnTerminate=false/FreeAndNil()
. In the third test, I got rid of the WaitFor()
without success. In all cases, the app freezes with message:
Project1 isn't responding. Do you want to close it ? Wait/OK
Note that in the third scenario, the TAniIndicator
spun correctly if the variable PHONENumber
is assigned before the execution of the REST request. Curiously, after this line, the app freezes again. This would be my preferred solution.
Below is my code:
.h
// ...
//---------------------------------------------------------------------------
class TAlphaThread : public TThread
{private:
protected:
void __fastcall Execute();
public:
__fastcall TAlphaThread(bool CreateSuspended);
//void __fastcall OnTerminate(TObject *Sender); Never triggered
};
//---------------------------------------------------------------------------
//===========================================================================
//---------------------------------------------------------------------------
class TBetaThread : public TThread
{private:
protected:
void __fastcall Execute();
public:
__fastcall TBetaThread(bool CreateSuspended);
//void __fastcall OnTerminate(TObject *Sender); Never triggered
};
//---------------------------------------------------------------------------
//===========================================================================
//---------------------------------------------------------------------------
class TDeltaThread : public TThread
{private:
protected:
void __fastcall Execute();
public:
__fastcall TDeltaThread(bool CreateSuspended);
//void __fastcall OnTerminate(TObject *Sender); Never triggered
};
//---------------------------------------------------------------------------
//===========================================================================
//---------------------------------------------------------------------------
class TForm1 : public TForm
{
__published: // Composants gérés par l'EDI
// ...
private: // Déclarations utilisateur
public: // Déclarations utilisateur
__fastcall TForm1(TComponent* Owner);
UnicodeString PHONENumber; // Mobile number
TRESTClient *REST1Client;
TRESTRequest *REST1Request;
TRESTResponse *REST1Response;
TAlphaThread *AlphaThread;
void __fastcall AlphaThreadTerminated(TObject *Sender);
TRESTClient *REST2Client;
TRESTRequest *REST2Request;
TRESTResponse *REST2Response;
TBetaThread *BetaThread;
void __fastcall BetaThreadTerminated(TObject *Sender);
TRESTClient *REST3Client;
TRESTRequest *REST3Request;
TRESTResponse *REST3Response;
TDeltaThread *DeltaThread;
void __fastcall DeltaThreadTerminated(TObject *Sender);
};
//---------------------------------------------------------------------------
//...
.cpp
//...
TForm1 *Form1;
//---------------------------------------------------------------------------
__fastcall TForm1::TForm1(TComponent* Owner)
: TForm(Owner)
{//
REST1Client = new TRESTClient(this);
REST1Request = new TRESTRequest(this);
REST1Response = new TRESTResponse(this);
REST2Client = new TRESTClient(this);
REST2Request = new TRESTRequest(this);
REST2Response = new TRESTResponse(this);
REST3Client = new TRESTClient(this);
REST3Request = new TRESTRequest(this);
REST3Response = new TRESTResponse(this);
}
//---------------------------------------------------------------------------
//===========================================================================
// Scenario 1 => Project1 isn't responding. Do you want to close it ? Wait/OK
//---------------------------------------------------------------------------
void __fastcall TForm1::Button1Tap(TObject *Sender, const TPointF &Point)
{//
PHONENumber = NULL;
AlphaThread = new TAlphaThread(true);
AlphaThread->OnTerminate = &AlphaThreadTerminated;
AlphaThread->Start();
if(AlphaThread->WaitFor())
{if(PHONENumber == NULL) ShowMessage(L"Null value!"); else Label1->Text = PHONENumber;
}
}
//---------------------------------------------------------------------------
void __fastcall TForm1::AlphaThreadTerminated(TObject * Sender)
{//
FreeAndNil(Form1->AlphaThread);
}
//---------------------------------------------------------------------------
__fastcall TAlphaThread::TAlphaThread(bool CreateSuspended) : TThread(CreateSuspended)
{//
FreeOnTerminate = false;
}
//---------------------------------------------------------------------------
/* Never triggered
void __fastcall TAlphaThread::OnTerminate(TObject * Sender)
{ShowMessage(L"TAlphaThread::OnTerminate!");
FreeAndNil(Form1->AlphaThread);
}*/
//---------------------------------------------------------------------------
void __fastcall TAlphaThread::Execute()
{//
Synchronize([this](){
Form1->AniIndicator1->Align = TAlignLayout::Contents;
Form1->AniIndicator1->Enabled = true;
Form1->AniIndicator1->Visible = true;
}
);
// Chargement des données
Form1->REST1Client->BaseURL = "my.URL.php";
Form1->REST1Request->AddParameter("ID", "123");
Form1->REST1Request->Client = Form1->REST1Client;
Form1->REST1Request->Response = Form1->REST1Response;
Form1->REST1Response->ContentType = "application/json";
Form1->REST1Request->Execute();
// Assignation des variables
TJSONValue *JV0TOPLevel; JV0TOPLevel = Form1->REST1Response->JSONValue;
TJSONArray *JSONArray = dynamic_cast<TJSONArray*>(JV0TOPLevel);
TJSONObject *JSONObject = dynamic_cast<TJSONObject*>(JSONArray->Items[0]);
TJSONValue *JV0ITEMLevel;
JV0ITEMLevel = JSONObject->GetValue("FIELDName"); Form1->PHONENumber = JV0ITEMLevel->AsType<String>();
Synchronize([this](){
Form1->AniIndicator1->Enabled = false;
Form1->AniIndicator1->Visible = false;
}
);
ReturnValue = 1;
Terminate();
}
//---------------------------------------------------------------------------
//===========================================================================
// Scenario 2 => Project1 isn't responding. Do you want to close it ? Wait/OK
//---------------------------------------------------------------------------
void __fastcall TForm1::Button2Tap(TObject *Sender, const TPointF &Point)
{//
PHONENumber = NULL;
BetaThread = new TBetaThread(true);
BetaThread->OnTerminate = &BetaThreadTerminated;
BetaThread->Start();
if(BetaThread->WaitFor())
{if(PHONENumber == NULL) ShowMessage(L"Null value!"); else Label2->Text = PHONENumber;
}
}
//---------------------------------------------------------------------------
void __fastcall TForm1::BetaThreadTerminated(TObject * Sender)
{//
FreeAndNil(Form1->BetaThread);
}
//---------------------------------------------------------------------------
__fastcall TBetaThread::TBetaThread(bool CreateSuspended)
: TThread(CreateSuspended)
{//
FreeOnTerminate = false;
}
//---------------------------------------------------------------------------
/*
void __fastcall TBetaThread::OnTerminate(TObject * Sender)
{// Never triggered
ShowMessage(L"TBetaThread::OnTerminate!");
FreeAndNil(Form1->BetaThread);
}*/
//---------------------------------------------------------------------------
void __fastcall TBetaThread::Execute()
{//
Queue(NULL, [this](){
Form1->AniIndicator1->Align = TAlignLayout::Contents;
Form1->AniIndicator1->Enabled = true;
Form1->AniIndicator1->Visible = true;
}
);
// Chargement des données
Form1->REST2Client->BaseURL = "my.URL.php";
Form1->REST2Request->AddParameter("ID", "123");
Form1->REST2Request->Client = Form1->REST2Client;
Form1->REST2Request->Response = Form1->REST2Response;
Form1->REST2Response->ContentType = "application/json";
Form1->REST2Request->Execute();
// Assignation des variables
TJSONValue *JV0TOPLevel; JV0TOPLevel = Form1->REST2Response->JSONValue;
TJSONArray *JSONArray = dynamic_cast<TJSONArray*>(JV0TOPLevel);
TJSONObject *JSONObject = dynamic_cast<TJSONObject*>(JSONArray->Items[0]);
TJSONValue *JV0ITEMLevel;
JV0ITEMLevel = JSONObject->GetValue("FIELDName"); Form1->PHONENumber = JV0ITEMLevel->AsType<String>();
Queue(NULL, [this](){
Form1->AniIndicator1->Enabled = false;
Form1->AniIndicator1->Visible = false;
}
);
ReturnValue = 1;
Terminate();
}
//---------------------------------------------------------------------------
//===========================================================================
// Scenario 3 => Project1 isn't responding. Do you want to close it ? Wait/OK
//---------------------------------------------------------------------------
void __fastcall TForm1::Button3Tap(TObject *Sender, const TPointF &Point)
{//
PHONENumber = NULL;
DeltaThread = new TDeltaThread(true);
/*DeltaThread->OnTerminate = &DeltaThreadTerminated;*/
DeltaThread->Start();
while(PHONENumber == NULL) {
[this]()->String{return PHONENumber;};
}
if(PHONENumber == NULL) ShowMessage(L"Null value!"); else Label3->Text = PHONENumber;
}
//---------------------------------------------------------------------------
void __fastcall TForm1::DeltaThreadTerminated(TObject * Sender)
{//
FreeAndNil(Form1->DeltaThread);
}
//---------------------------------------------------------------------------
__fastcall TDeltaThread::TDeltaThread(bool CreateSuspended)
: TThread(CreateSuspended)
{//
FreeOnTerminate = true;
}
//---------------------------------------------------------------------------
/* //Never triggered
void __fastcall TDeltaThread::OnTerminate(TObject * Sender)
{//ShowMessage(L"OnTerminate!");
//FreeAndNil(Form1->DeltaThread);
} */
//---------------------------------------------------------------------------
void __fastcall TDeltaThread::Execute()
{//
//Form1->PHONENumber = 123; Working!!
Queue(NULL, [this](){
Form1->AniIndicator1->Align = TAlignLayout::Contents;
Form1->AniIndicator1->Enabled = true;
Form1->AniIndicator1->Visible = true;
}
);
//Form1->PHONENumber = 123; Working!!
// Chargement des données
Form1->REST3Client->BaseURL = "my.URL.php";
Form1->REST3Request->AddParameter("ID", "123");
Form1->REST3Request->Client = Form1->REST3Client;
Form1->REST3Request->Response = Form1->REST3Response;
Form1->REST3Response->ContentType = "application/json";
Form1->REST3Request->Execute();
// Form1->PHONENumber = 123; Freezing
// Assignation des variables
TJSONValue *JV0TOPLevel; JV0TOPLevel = Form1->REST3Response->JSONValue;
TJSONArray *JSONArray = dynamic_cast<TJSONArray*>(JV0TOPLevel);
TJSONObject *JSONObject = dynamic_cast<TJSONObject*>(JSONArray->Items[0]);
TJSONValue *JV0ITEMLevel;
JV0ITEMLevel = JSONObject->GetValue("FIELDName"); Form1->PHONENumber = JV0ITEMLevel->AsType<String>(); // Freezing
Queue(NULL, [this](){
Form1->AniIndicator1->Enabled = false;
Form1->AniIndicator1->Visible = false;
}
);
ReturnValue = 1;
Terminate();
}
//---------------------------------------------------------------------------
Any idea of what I missed?
CodePudding user response:
Scenario 1:
TThread::WaitFor()
is blocking the main UI thread until the thread finishes running.WaitFor()
does not process new GUI messages while waiting, but it does process pendingSynchronize()
andQueue()
requests (and does dispatch cross-thread messages sent withSendMessage...()
to avoid deadlocks).you can't free the thread object from inside of its
OnTerminated
event handler. The RTL still needs access to the thread object after the handler exits. If you want the thread to free itself, set itsFreeOnTerminate
property totrue
instead. However, in this scenario, since you are waiting on the thread, you could just free the thread afterWaitFor()
exits instead.
A better way to handle this scenario is to update the TAniIndicator
directly in Button1Tap()
and get rid of the OnTerminate
event handler altogether. The TAniIndicator
logic doesn't really belong in the thread's Execute()
method to begin with. However, you will have to manually pump the main thread's message queue for new messages while waiting for the thread to finish.
void __fastcall TForm1::Button1Tap(TObject *Sender, const TPointF &Point)
{
PHONENumber = _D("");
AlphaThread = new TAlphaThread(false);
AniIndicator1->Align = TAlignLayout::Contents;
AniIndicator1->Enabled = true;
AniIndicator1->Visible = true;
HANDLE h = (HANDLE) AlphaThread->Handle;
while (MsgWaitForMultipleObjects(1, &h, FALSE, INFINITE, QS_ALLINPUT) == (WAIT_OBJECT_0 1))
{
Application->ProcessMessages();
}
int result = AlphaThread->WaitFor();
delete AlphaThread;
AniIndicator1->Enabled = false;
AniIndicator1->Visible = false;
if (result)
{
if(PHONENumber == _D(""))
ShowMessage(_D("Null value!"));
else
Label1->Text = PHONENumber;
}
}
//---------------------------------------------------------------------------
__fastcall TAlphaThread::TAlphaThread(bool CreateSuspended) : TThread(CreateSuspended)
{
FreeOnTerminate = false;
}
//---------------------------------------------------------------------------
void __fastcall TAlphaThread::Execute()
{
// Chargement des données
Form1->REST1Client->BaseURL = _D("my.URL.php");
Form1->REST1Request->AddParameter(_D("ID"), _D("123"));
Form1->REST1Request->Client = Form1->REST1Client;
Form1->REST1Request->Response = Form1->REST1Response;
Form1->REST1Response->ContentType = _D("application/json");
Form1->REST1Request->Execute();
// Assignation des variables
TJSONValue *JV0TOPLevel = Form1->REST1Response->JSONValue;
TJSONArray *JSONArray = static_cast<TJSONArray*>(JV0TOPLevel);
TJSONObject *JSONObject = static_cast<TJSONObject*>(JSONArray->Items[0]);
TJSONValue *JV0ITEMLevel = JSONObject->GetValue(_D("FIELDName"));
Form1->PHONENumber = JV0ITEMLevel->AsType<String>();
ReturnValue = 1;
}
However, in this scenario, there is really no point whatsoever in having the main thread create a worker thread just to block itself waiting on that thread. You may as well just perform the REST logic directly in the main thread instead. You can use the REST components asynchronously to avoid blocking the main thread (ie, by using TRESTRequest::ExecuteAsync()
instead of TRESTRequest::Execute()
).
Scenario 2:
- all of the same issues as Scenario 1.
Scenario 3:
there is no call to
TThread::WaitFor()
in the main thread, so theQueue()
requests go unprocessed, but at least they won't block the worker thread from running. That would not be the case had you usedSynchronize()
instead.the
while
loop in the main thread is creating a new lambda on each iteration, but is not actually calling the lambda. This is just a busy loop that eats up CPU cycles for no gain.same issue with freeing the thread as the other scenarios.
Now, with all of that said, the correct way to handle this situation using a worker thread is to not wait on the thread at all. Let it run normally, and let it notify the main thread when finished. DO NOT block the main thread at all in the meantime.
Try this:
class TGetPhoneNumberThread : public TThread
{
protected:
void __fastcall Execute();
public:
__fastcall TGetPhoneNumberThread(bool CreateSuspended);
String PHONENumber;
};
void __fastcall TForm1::Button1Tap(TObject *Sender, const TPointF &Point)
{
TGetPhoneNumberThread *Thread = new TGetPhoneNumberThread(true);
Thread->OnTerminate = &GetPhoneNumberThreadTerminated;
Thread->Start();
Button1->Enabled = false;
AniIndicator1->Align = TAlignLayout::Contents;
AniIndicator1->Enabled = true;
AniIndicator1->Visible = true;
}
//---------------------------------------------------------------------------
void __fastcall TForm1::GetPhoneNumberThreadTerminated(TObject *Sender)
{
AniIndicator1->Enabled = false;
AniIndicator1->Visible = false;
Button1->Enabled = true;
TGetPhoneNumberThread *Thread = static_cast<TGetPhoneNumberThread*>(Sender);
if (Thread->FatalException)
ShowMessage(_D("Error! ") static_cast<Exception*>(Thread->FatalException)->Message);
else if (Thread->PHONENumber == _D(""))
ShowMessage(_D("Null value!"));
else
Label1->Text = Thread->PHONENumber;
}
}
//---------------------------------------------------------------------------
__fastcall TGetPhoneNumberThread::TGetPhoneNumberThread(bool CreateSuspended) : TThread(CreateSuspended)
{
FreeOnTerminate = true;
}
//---------------------------------------------------------------------------
#include <memory>
void __fastcall TGetPhoneNumberThread::Execute()
{
std::unique_ptr<TRESTClient> RESTClient(new TRESTClient(NULL));
std::unique_ptr<TRESTRequest> RESTRequest(new TRESTRequest(NULL));
std::unique_ptr<TRESTResponse> RESTResponse(new TRESTResponse(NULL));
// Chargement des données
RESTClient->BaseURL = _D("my.URL.php");
RESTRequest->AddParameter(_D("ID"), _D("123"));
RESTRequest->Client = RESTClient.get();
RESTRequest->Response = RESTResponse.get();
RESTResponse->ContentType = _D("application/json");
RESTRequest->Execute();
// Assignation des variables
TJSONValue *JV0TOPLevel = RESTResponse->JSONValue;
TJSONArray *JSONArray = static_cast<TJSONArray*>(JV0TOPLevel);
TJSONObject *JSONObject = static_cast<TJSONObject*>(JSONArray->Items[0]);
TJSONValue *JV0ITEMLevel = JSONObject->GetValue(_D("FIELDName"));
PHONENumber = JV0ITEMLevel->AsType<String>();
}
Alternatively, you can get rid of the worker thread completely and use TRESTRequest
asynchronously instead:
void __fastcall TForm1::Button1Tap(TObject *Sender, const TPointF &Point)
{
// Chargement des données
REST1Client->BaseURL = _D("my.URL.php");
REST1Request->AddParameter(_D("ID"), _D("123"));
REST1Request->Client = REST1Client;
REST1Request->Response = REST1Response;
REST1Response->ContentType = _D("application/json");
REST1Request->ExecuteAsync(
[this](){
// Assignation des variables
TJSONValue *JV0TOPLevel = REST1Response->JSONValue;
TJSONArray *JSONArray = static_cast<TJSONArray*>(JV0TOPLevel);
TJSONObject *JSONObject = static_cast<TJSONObject*>(JSONArray->Items[0]);
TJSONValue *JV0ITEMLevel = JSONObject->GetValue(_D("FIELDName"));
PHONENumber = JV0ITEMLevel->AsType<String>();
TThread::Queue(nullptr,
[this](){
AniIndicator1->Enabled = false;
AniIndicator1->Visible = false;
Button1->Enabled = true;
if (PHONENumber == _D(""))
ShowMessage(_D("Null value!"));
else
Label1->Text = PHONENumber;
}
);
}
},
false
);
Button1->Enabled = false;
AniIndicator1->Align = TAlignLayout::Contents;
AniIndicator1->Enabled = true;
AniIndicator1->Visible = true;
}
CodePudding user response:
Synchronize()
will block until the code has been executed in the main thread, i.e. messages have been processed in the main thread. Because Windows tells you the application is unresponsive, I think WaitFor()
does not process messages while waiting.
Instead of WaitFor()
, use a loop that calls Application->ProcessMessages()
:
while (!AlphaThread->CheckTerminated()) {
Application->ProcessMessages();
Sleep(1);
}
If you are waiting for the thread, you cannot free it in OnTerminate()
because then CheckTerminated()
could result to access violation. Instead, free it after the waiting has finished. In C , you can simply store the thread in std::unique_ptr
to have it automatically freed. As the variable is not needed after the button handler finishes, it can be a local variable.
Something like this should work:
auto AlphaThread = std::unique_ptr<TAlphaThread>(new TAlphaThread(true));
AlphaThread->Start();
while (!AlphaThread->CheckTerminated()) {
Application->ProcessMessages();
Sleep(1);
}
Be aware that you are using Synchronize()
and Queue()
unnecessarily: You could take that code out and execute it in the button handler instead. You can enable the indicator before starting the thread, and disable it after the thread has finished.