I'd like to ask some advice on how handle an Embarcadero CB10.1 issue with re-entrancy. Compiled in Debug configuration with "Disable all optimizations" set to true. I'm running on Win7.
I have a simple test case. A form with two buttons. The OnClick event handler for each button calls the same CPU intensive function. Below is the header file followed by the program file.
#ifndef Unit1H
#define Unit1H
//---------------------------------------------------------------------------
#include <System.Classes.hpp>
#include <Vcl.Controls.hpp>
#include <Vcl.StdCtrls.hpp>
#include <Vcl.Forms.hpp>
//---------------------------------------------------------------------------
class TForm1 : public TForm
{
__published: // IDE-managed Components
TButton *Button1;
TButton *Button2;
void __fastcall Button1Click(TObject *Sender);
void __fastcall Button2Click(TObject *Sender);
private: // User declarations
double __fastcall CPUIntensive(double ButonNo);
double __fastcall Spin(double Limit);
public: // User declarations
__fastcall TForm1(TComponent* Owner);
};
//---------------------------------------------------------------------------
extern PACKAGE TForm1 *Form1;
//---------------------------------------------------------------------------
#endif
//---------------------------------------------------------------------------
#include <vcl.h>
#pragma hdrstop
#include "Unit1.h"
//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
TForm1 *Form1;
//---------------------------------------------------------------------------
__fastcall TForm1::TForm1(TComponent* Owner)
: TForm(Owner)
{
}
//---------------------------------------------------------------------------
void __fastcall TForm1::Button1Click(TObject *Sender)
{
Button1->Caption = "Pushed";
double retv = CPUIntensive(1);
Button1->Caption = "Button1";
if (retv) ShowMessage("Button1 Done");
}
//---------------------------------------------------------------------------
void __fastcall TForm1::Button2Click(TObject *Sender)
{
Button2->Caption = "Pushed";
double retv = CPUIntensive(2);
Button2->Caption = "Button2";
if (retv) ShowMessage("Button2 Done");
}
//---------------------------------------------------------------------------
double __fastcall TForm1::CPUIntensive(double ButtonNo)
{
//
static bool InUse = false;
if (InUse) {
ShowMessage("Reentered by button number " + String(ButtonNo));
while (InUse) {};
}
double retv;
InUse = true;
retv = Spin(30000); // about 9 seconds on my computer
//retv += Spin(30000); // uncomment if you have a faster computer
//retv += Spin(30000);
InUse = false;
return retv;
}
//---------------------------------------------------------------------------
double __fastcall TForm1::Spin(double Limit)
{
double k;
for (double i = 0 ; i < Limit ; i++) {
for (double j = 0 ; j < Limit ; j++) {
k = i + j;
// here there can be calls to other VCL functions
Application->ProcessMessages(); // added so UI would be responsive (2nd case)
}
}
return k;
}
//---------------------------------------------------------------------------
- 1st case : the code shown but WITHOUT the call to ProcessMessages().
When I run this and click on button 1, CPU usage jumps up to almost 100% for about 9 seconds. The form becomes unresponsive during this time. Can't move the form or click on button 2.
That works as I would expect.
2nd case : To make the form responsive to the user during the CPU intensive function, I added the ProcessMessages() call as shown. Now, I can move the form around and click on other buttons.
That is not always good, because I could click on button 1 again or even click on button 2. Either click would fire off the CPU intensive function again. To prevent the CPU intensive function from running the second time, I made a static boolean flag "InUse". I set that when the function starts and clear it when the function completes.
So I check the flag when I enter the CPU intensive function and if its set (it must have been set by a previous click on a button), I show a message and then wait for the flag to clear.
But the flag never clears and my program loops on the 'while' statement forever. I would like the program to just wait for the CPU intensive function to complete and then just run it again.
If I set a breakpoint in the Spin() function after I hit the deadlock, it never fires indicating that neither event is executing.
I know the VCL is not thread safe but here, all of the processing takes place in the main thread. In my actual code, there are many calls to VCL functions so the CPU intensive function has to remain in the main thread.
I considered Critical Sections and Mutexes but since everything is in the main thread, any use of them does no blocking.
Maybe its a stack issue? Is there a solution that lets me handle this without the deadlock?
That is always the wrong solution. The correct way to handle this situation is to move the CPU intensive code to a separate worker thread, and then have your button events start a new instance of that thread if it is not already running. Or, keep the thread running in a loop that sleeps when it doesn't have work to do, and then have each button event signal the thread to wake up and do its work. Either way, NEVER block the main UI thread!
A better way would be to disable the buttons while the work is being performed, and re-enable them when finished. Then the work can't be re-entered to begin with.
But, even if you keep your flag, your function should just exit without doing anything if the flag is already set.
Either way, you should display a UI displayed telling the user when the work is in progress. This becomes easier to manage if the work is done in a separate thread.
That is because you are just running an endless loop that does nothing, so it doesn't allow the code to progress any further. And certainly not finish the existing work and reset the flag.
The smallest fix you can make to your existing code without re-writing it is to change
CPUIntensive()
to usereturn 0
instead ofwhile (InUse) {}
whenInUse
is true. That will allow a call toProcessMessages()
to exit and return control back to the previousCPUIntensive()
call that is waiting to finish running.Thay is a BIG mistake.
That is not a good enough reason to perform the work in the main thread. Move it to a worker thread, where it belongs, and have it sync with the main thread whenever it needs to access the UI. Do as much work as possible in the worker thread, and sync only when absolutely necessary.