Embarcadero CoCreateInstance exception 0x40000015

[rlebeau]

I think I understand that the first and second solutions are equivalent

Yes, they are.

and MyFunction () does not generate race conditions if I instantiate 1000 threads, right?

No. Both solutions have a very serious race condition - it is NOT safe to access GUI controls in worker threads without synchronizing access with the main UI thread. You must do something more like the following instead:

Code: Select all

class TMyThread : public TThread
{
private:
    String LabelStr;
    void __fastcall UpdateLabel();
protected:
    void __fastcall Execute();
    ...
};
    
void __fastcall TMyThread::UpdateLabel()
{
    Form1->Label1->Caption = LabelStr;
}

void __fastcall TMyThread::Execute()
{
    for (int i = 0; i < 100; ++i)
    {
        LabelStr = IntToStr(i);
        Synchronize(&UpdateLabel);
    }
}

Or, in the C++11 compilers, you can use a lambda instead of a class method:

Code: Select all

void __fastcall TMyThread::Execute()
{
    for (int i = 0; i < 100; ++i)
    {
        Synchronize([=](){ Form1->Label1->Caption = IntToStr(i); });
    }
}

[mark_c]

thanks Remy.
What I wanted to understand is if there is a race condition calling MyFunction () or does each thread create its own private copy of MuFunction ()?
Sorry but I was wrong to put the Label, I shouldn't have put it.

This time I removed the Label for clarity and as you had already explained to me that there is a race condition between the threads and the components of the GUI. Here I am interested in knowing if there is some problem programmatically or if the two solutions are equivalent?

first solution

Code: Select all

void __fastcall TMyThread :: Execute ()
{
        for (int i = 0; i <1000000; i ++);
}

second solution

Code: Select all

void __fastcall TMyThread :: Execute ()
{
       MyFunction ();
}


void MyFunction ()
{
        for (int i = 0; i <1000000; i ++);
}

[rlebeau]

What I wanted to understand is if there is a race condition calling MyFunction () or does each thread create its own private copy of MuFunction ()?

That is not how functions work. There is only 1 copy of the function code in memory. But each thread has its own local call stack with which to use for making function calls (ie, to hold function parameters, return addresses, stack frames, etc). There is no race condition on simply calling a function in multiple threads, but if that function internally accesses any shared resources across thread boundaries then it needs to protect that access with proper thread synchronization.

This time I removed the Label for clarity

Your new examples have no race conditions, as everything is local to each thread, there are no shared resources being accessed across thread boundaries.

[mark_c]

sorry Remy,
and if I have this situation:

Code: Select all

int threadNumber;

void __fastcall TMyThread :: Execute ()
{
            threadNumber ++;
            Synchronize (& MyLabel);

            for (int i = 0; i <1000000; i ++);
}

and I create 1000 threads, is there still race condition?

[rlebeau]

and if I have this situation:
...
and I create 1000 threads, is there still race condition?

Yes, because now you have a global variable (threadNumber) that is being shared across thread boundaries, so you need to protect it from concurrent access. The easiest way to do that in this example is to use the Interlocked API, either with InterlockedIncrement() or TInterlocked::Increment().

[mark_c]

thanks Remy.
You will find lines of code that are familiar to you as you taught them to me.
I ask you once again if you see race condition issue if you were to instantiate 100 threads.

Note that bfn [] is only read by threads, never written or modified.

Code: Select all

//---------------------------------------------------------------------------

#include <vcl.h>
#pragma hdrstop

#include "Unit1.h"

class TMyThread : public TThread
{
private:
	int buffer_Num;
protected:
	void __fastcall Execute();
public:
	__fastcall TMyThread(int &threadNumber);
};

short *bfn[100];

//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
TForm1 *Form1;
//---------------------------------------------------------------------------
__fastcall TForm1::TForm1(TComponent* Owner)
        : TForm(Owner)
{
        for(int i=0; i<100; i++)
                bfn[i] = new short[10];

[b]//load data into buffers[/b]
        bfn[...][i] = ..........;
}
//---------------------------------------------------------------------------
__fastcall TMyThread::TMyThread(int &threadNumber)
: TThread(true), buffer_Num(threadNumber)
{
	FreeOnTerminate = true;
}
//---------------------------------------------------------------------------

void __fastcall TMyThread::Execute()
{
// [b]I use buffers for read only, never write[/b]

            for (int i = 0; i <100; i ++)
                bfn[buffer_Num][i];
}
//---------------------------------------------------------------------------

void __fastcall TForm1::Button1Click(TObject *Sender)
{
        TMyThread *Thread;

	try
	{
		for(int i = 0; i < 100; i++)
		{
			Thread = new TMyThread( i );
			Thread->Resume();
                        Sleep(100);
		}
	} catch(...){ }
}
//---------------------------------------------------------------------------

[rlebeau]

I ask you once again if you see race condition issue if you were to instantiate 100 threads.

This is not a hard idea to grasp. If multiple threads are sharing a variable, and at least 1 thread writes to that variable while any of the other threads are still making use of the variable, then you must protect the variable from concurrent access. Simple as that.

Note that bfn [] is only read by threads, never written or modified.

Then there is no race condition.

Code: Select all

__fastcall TMyThread(int &threadNumber);

Why are you passing the parameter by-reference? Inside the constructor body, you are assigning the parameter to another 'int' variable that is local to the TMyThread object only, thus you are making a copy of the variable's current value. The worker thread will not be accessing the original variable at all, so there is no point in passing it in by-reference if you are just going to copy it anyway. Pass it in by-value instead.

Code: Select all

__fastcall TMyThread(int threadNumber);

Code: Select all

short *bfn[100];
...
for(int i=0; i<100; i++)
	bfn[i] = new short[10];

Since you know the full array size at compile-time, why not just declare it statically instead? It it small enough that it won't take up much memory:

Code: Select all

short bfn[100][10];

Code: Select all

void __fastcall TMyThread::Execute()
{
	for (int i = 0; i <100; i ++)
		bfn[buffer_Num][i];
}

This is a buffer overflow error. The 2nd dimension of the array holds only 10 numbers, but you are accessing 100 from it, which is way out of bounds.

Now, this all being said, you can simply your thread code a little bit:

Code: Select all

//---------------------------------------------------------------------------

#include <vcl.h>
#pragma hdrstop

#include "Unit1.h"

class TMyThread : public TThread
{
private:
	short *buffer;
protected:
	void __fastcall Execute();
public:
	__fastcall TMyThread(short *buffer);
};

short bfn[100][10];

//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
TForm1 *Form1;
//---------------------------------------------------------------------------
__fastcall TForm1::TForm1(TComponent* Owner)
        : TForm(Owner)
{
	//load data into buffers
	bfn[...][i] = ..........;
}
//---------------------------------------------------------------------------
__fastcall TMyThread::TMyThread(short *buffer)
    : TThread(false), buffer(buffer)
{
	FreeOnTerminate = true;
}
//---------------------------------------------------------------------------
void __fastcall TMyThread::Execute()
{
	// I use buffers for read only, never write
	for (int i = 0; i < 10; ++i)
		buffer[i];
}
//---------------------------------------------------------------------------
void __fastcall TForm1::Button1Click(TObject *Sender)
{
	for(int i = 0; i < 100; ++i)
	{
		new TMyThread( &bfn[i] );
		Sleep(100);
	}
}
//---------------------------------------------------------------------------

[mark_c]

thanks Remy.