Help with CMutex and DB access

Bill Wilson

my project consists of 3 services and a set of COM objects. The COM objects each host a pool of worker threads. Each object has three interfaces, one for each service. The COM objects and the services all update the same table in a SQL Server DB. The multi-threaded objects post a message to a single thread to do their updates. Do I need a synchronization device, such as CMutex to keep these from stepping on each other? If so, please give me a specific recommendation. Thanks for the help, Bill

Joaquin M Lopez Munoz

It is just a question of how many threads (considering the whole project) access the same resource (in this case, the DB). If it's more than one, then you're better off having the resource protected by a named (interprocess) mutex. My recommendation is that you stay away from MFC wrappers around synchronization objects, as they're counterintuitive and in some cases broken. Native Win32 mutexes are not so hard to use directly. Here's a starter for a simple class that access a named mutex (or creates if needed), locks on it and releases the lock on destroy. The use is simple: just define a CMutexAccesor object at the beginning of each critizal zone. Use some unique string as lpstrMutexName (like your project name followed by a lot or garbage digits, or a GUID if you're into these). Warning: I've adapted this from some code of mine in Spanish, so some typos are likely to have been introduced.

class CMutexAccesor
{
public:
CMutexAccesor(LPCTSTR lpstrMutexName){
res=(mutex=CreateMutex(NULL,FALSE,lpstrMutexName))!=NULL;
if(res){
// Wait up to 4 secs for locking
res=WaitForSingleObject(mutex,4000)==WAIT_OBJECT_0;
if(!res)CloseHandle(mutex);
}
}
~CMutexAccesor(){
if(res){
ReleaseMutex(mutex);
CloseHandle(mutex);
}
}

BOOL res;

private:
HANDLE mutex;
};

Hope this helps. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

Bill Wilson

Thanks for the help. I have on more question. Here are two samples of what I believe you are telling me. I believe the first one is correcte, but would find it wonderful if the second one would also work. Thas tould let me simply override the Update method of my CRecordset objects. In this first case I am protecting the entire DB operation. There are literally hundreds of updates in the project. All are unprotected. :omg:

class CTester(prsSomeTable)
{
UpdateDB()
{
CMutexAccessor *pMut;
pMut = new CMutexAccessor();
prs->Edit();
prs->x = 2;
prs->Update();
delete pMut;
}
};

In the second case I am only protecting the Update. Is this sufficient? If I can get away with simply protecting the Update operation itself, I can reduce the problem to about a dozen changes by overriding the Update method of my recordset classes. :)

class CTester(prsSomeTable)
{
UpdateDB()
{
CMutexAccessor *pMut;

	prs->Edit();
	prs->x = 2;
	pMut = new CMutexAccessor();
	prs->Update();
	delete pMut;
}

};

Thanks for the help, Bill

Joaquin M Lopez Munoz

I couldn't tell from the little information you provide. What you have to determine is wether the operation is trying to change a shared resource. So: is that prs private to each thread? Is Edit() an read-only operation? If both answers are yes, I guess it is safe to leave those ops out of the mutex-protected zone. If in doubt, wrap it all up within the mutex and measure performance --maybe it is OK for your purposes. Another subject is that CMutexAccessor should not be used like you do, but this other way:

UpdateDB()
{
CMutexAccessor mut("MYONEANDONLYNAMENOONECAMETOFIGUREOUTBEFORE");
prs->Edit();
prs->x = 2;
pMut = new CMutexAccessor();
prs->Update();
}

I.e, construct it on the stack, not with new. This way you stay confident that the destructor will be called even if your function exits in unexpected fashions (for instance, if some of the operations throws an exception). To protect more or less code inside the function just move the CMutexAccessor mut... line up or down. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

Bill Wilson

Thanks again. Sorry I forgot to type in the mutex name. Here's what I actually came up with... (sample usage is at the bottom. in stdafx.h: extern CRequests * g_prsRequests; In main program module CRequests * g_prsRequests;g_prsRequests In program entry point: g_prsRequests = new CRequests(); Here is the modified CRequests class. What do you think?

class CRequests : public CRecordset
{
public:
CRequests(CDatabase* pDatabase = NULL);
DECLARE_DYNAMIC(CRequests)

// Field/Param Data
//{{AFX_FIELD(CRequests, CRecordset)
long m_ID;
CString m_SERVICE_TYPE;
clip for brevity
//}}AFX_FIELD
CMutexAccesor *pMutex;
#define DBMUTEX "AFX_REQUESTS_3B2738B5_A172_11D5_9FCB_00B0D081C96F"

// Overrides
// ClassWizard generated virtual function overrides
//{{AFX_VIRTUAL(CRequests)
public:
virtual CString GetDefaultConnect(); // Default connection string
virtual CString GetDefaultSQL(); // Default SQL for Recordset
virtual void DoFieldExchange(CFieldExchange* pFX); // RFX support
//}}AFX_VIRTUAL
virtual void AddNew()
{
try
{
pMutex = new CMutexAccesor(DBMUTEX);
CRecordset::AddNew();
}
catch (CDBException *e)
{
if (pMutex)
{
if (pMutex) delete pMutex;
pMutex = NULL;
}
throw e;
}
if (pMutex) delete pMutex;

}

virtual void Edit()
{
	try
	{
		pMutex = new CMutexAccesor(DBMUTEX);
		CRecordset::Edit();
	}
	catch (CDBException \*e)
	{
		if (pMutex)
		{
			if (pMutex) delete pMutex;
			pMutex = NULL;
		}
		throw e;
	}
	if (pMutex) delete pMutex;
}

virtual BOOL Update()
{
	BOOL bRet;
	try
	{
		bRet = CRecordset::Update();
	}
	catch (CDBException \*e)
	{
		delete pMutex;
		throw e;
	}
	delete pMutex;
	return bRet;
}

void AbandonUpdate()
{
	if (pMutex) delete pMutex;
}

};

in the cpp file I set pMutex = NULL in the constructor and delete it in the desctructor. I'm using a pointer variable so I can create and destroy the mutex for each update operation. e.g. try { g_prsRequests->Edit(); // mutex created g_prsRequests->m_ID = 1; g_prsRequests->Update(); // mutext destroyed. } catch (CException *e) { ...blah blah in the case of an actual DB exception, the mutex is already destroyed... g_prsRequests->AbaondonUpdate(); // mutex destroyed } Thanks for the help, Bill

Joaquin M Lopez Munoz

No no no no no. You got it wrong. Do not have a CMutexAccessor pointer as a member of CRequests. Do not. This try catch labyrinth is something you would regret having to maintain in the long term. Instead, have a local CMutexAccessor variable at the beginning of each protected zone, like this:

virtual void AddNew()
{
CMutexAccesor(DBMUTEX) mut;
CRecordset::AddNew();
}
virtual void Edit(){
CMutexAccesor(DBMUTEX) mut;
CRecordset::Edit();
}

and so forth. That simple. This schema ensures exclusive access among sections of code adorned this way, no matter the object, the method, the thread or the process that piece of code belongs in. What identifies the section (and I guess this is where your misunderstanding stems from) is not the particular CMutexAccesor object used to perform the locking, but only the name of the mutex (DBMUTEX in your example). Do not worry also about mutex-protected methods calling other mutex-protected methods: it'll all work like a breeze (your approach is a maintenance nightmare with respect to this). Hope I made myself clear. Do't hesitate to ask for more help, if needed. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

Bill Wilson

Got it. :-O Thanks. You are right, I was thinking that the object held the lock, not the name. I really appreciate your time on this. You have likely saved me many hours of frustration. Thanks for the help, Bill