Enforcing single thread in MFC ActiveX using Mutex

[Dawoodoz]

I have to make thread safety for my MFC ActiveX graphics engine because Windows 7 allow an application to run with a messagebox waiting for the user and even allowing the messagebox to be in the background while the user continue to use the program and make new calls to the engine. Without thread safety, the engine crashes with access violation.

The following mutex code did absolutely nothing and I don't understand why.

// Declared in public class data
HANDLE EngineMutex;

// Executed in the class constructor
EngineMutex = CreateMutex(NULL,FALSE,"EngineAccess");

// Used in start and end of all interface calls
#define START_CALL WaitForSingleObject(EngineMutex,INFINITE);
#define END_CALL ReleaseMutex(EngineMutex);

[Dawoodoz]

It seems to be better to just put all error messages in a queue and wait for the calling application to read them.

[dexblack]

Are you protecting the right resource with your mutex?

[Dawoodoz]

I use the whole engine as a resource so that the caller should only be allowed to use one method at a time. My new error message system makes it safe to call from VB6 but a multicore application could still make the engine crash by having overlapping calls.

[Arjay]

Can you post the code for a few of the methods so we can see how you are protecting them?

[Dawoodoz]

I use the macros START_CALL and END_CALL in each area that reads or writes using the engine's shared data but the last time I tried, more than 10 threads was in the critical section at once and caused the engine to crash in 80% of the cases.

void CDFPGECtrl::Camera_RenderScene(int InputCamera, int OutputSurface, int ShaderChannel) {
	START_CALL
		GET_FROM_REF(Camera,InputCamera)
		GET_FROM_REF(DrawSurface,OutputSurface)
		if BAD_REF(InputCamera) {
			REPORT_TYPE(Camera,Camera_RenderScene,InputCamera)
		} else if BAD_REF(OutputSurface) {
			REPORT_TYPE(DrawSurface,Camera_RenderScene,OutputSurface)
		} else if (pOutputSurface->DepthBuffer == false) {
			MQ->InsertMessage(L"Camera_RenderScene: The draw surface reference OutputSurface does not have a depth buffer.");
		} else {
			DGE->Camera_RenderScene(pInputCamera,pOutputSurface,ShaderChannel);
		}
	END_CALL
}

int CDFPGECtrl::Model_CreateCopy(int SourceModel) {
	int Result;
	START_CALL
		GET_FROM_REF(Model,SourceModel)
		if BAD_REF(SourceModel) {
			REPORT_TYPE(Model,Model_CreateCopy,SourceModel)
			Result = -1;
		} else {
			Result = DGE->IDFromPointer(DGE->Model_DuplicateModel(pSourceModel));
		}
	END_CALL
	return Result;
}

[Arjay]

What is the code for START_CALL and END_CALL?

[Dawoodoz]

They are declared in the first post.

[Arjay]

Okay, so these locks are on a class instance level. This is fine if you are trying to prevent an instance of a class from calling more than one method in different threads.

If you need to synchronize across multiple class instances, but the functionality into a singleton class and just forward the method calls to the singleton class.

Something like:

void CDFPGECtrl::Camera_RenderScene(int InputCamera, int OutputSurface, int ShaderChannel)
{
  // forward the call to the singleton instance
  Service::GetInstance( )->Camera_RenderScene( InputCamera, OutputSurface, ShaderChannel );
}

[Dawoodoz]

If you need to synchronize across multiple class instances, but the functionality into a singleton class and just forward the method calls to the singleton class.

I don't need synchronization between different class instances because I can run 2 instances of the engine at the same time without collisions.

[Arjay]

I don't need synchronization between different class instances because I can run 2 instances of the engine at the same time without collisions.

If that is the case, then your previous statement appears to be incorrect: "I use the macros START_CALL and END_CALL in each area that reads or writes using the engine's shared data but the last time I tried, more than 10 threads was in the critical section at once and caused the engine to crash in 80% of the cases."

[Arjay]

If that is the case, then your previous statement appears to be incorrect: "I use the macros START_CALL and END_CALL in each area that reads or writes using the engine's shared data but the last time I tried, more than 10 threads was in the critical section at once and caused the engine to crash in 80% of the cases."

It appears that your engine isn't thread safe (and instances of the engine can't be called across multiple threads).

[Dawoodoz]

With shared data, I mean outside of the called function and not global memory.

[Dawoodoz]

It appears that your engine isn't thread safe (and instances of the engine can't be called across multiple threads).

Yes, I wrote that in the first post.

[Arjay]

Yes, I wrote that in the first post.

So put the engine into a singleton class and protect it as I've suggested.