CMutex::Lock vs. CSingleLock::Lock

Question

I've been tapped to support some legacy code, and I'm seeing some things that cause me to scratch my head in confusion. In some sections of code, I see that a class instance uses a CMutex instance to synchronize method execution. For instance

class CClassA : public CObject
{
public:
   void DoSomething();

private:
   CMutex m_mutex;
}

void CClassA::DoSomething()
{
   m_mutex.Lock();

   //...logic...

   m_mutex.Unlock();
}

Elsewhere in the same project I find that the code is using a CSingleLock

class CClassB : public CObject
{
public:
   void DoSomething();

private:
   CCriticalSection m_crit;
}

void CClassB::DoSomething()
{
   CSingleLock lock(&m_crit);
   lock.Lock();

   //...logic...

   lock.Unlock();
}

After reviewing MSDN documentation for synchronization, it would appear that CClassB is implementing the advised method, but it isn't clear to me what the danger is in the implementation used by CClassA. As far as I can tell, the only difference between the two methods is that CSingleLock has the benefit of RAII, so the lock is automatically released when execution exits scope. Are there any other benefits / drawbacks to either implementation?

Solution 1

In general mutexes can be used to control thread access across processes via a named mutex while critical sections are only for synchronizing thread access in the same process space.

Neither of these classes truly get the benefit of RAII without wrapping them because in that case you would never need to explicitly call lock or unlock. Take for example this little bit of pseudo code using a boost mutex lock ...

void DoSomething()
{
  // construction acquires lock on mutex
  boost::scoped_lock lock(&aBoostMutex);

  // ...

} // end scope - object is destroyed and lock is released

Now I'd argue you should avoid CMutex, CCritalSection, CSemaphore, and CEvent because the implementations are somewhat broken or at the very least inferior to other available libraries like boost. For example:

• Determining between a timeout from an abandoned mutex is impossible because the implementation checks return value only not the reason.
• No reentrant locks using CSingleLock so recursion will cause issues.
• Inability to have a named event between processes

Depending on what you are tasked with you might have the opportunity to move away from the MFC wrappers on windows API and either implement your own atomic locks or use something like boost or C++0x features like std::mutex which not only are better implementations but provide cross-platform support.

Solution 2

A critical section is only visible to/usable by the threads inside a single process. A mutex can be made visible across a number of processes (typically by creating a named mutex). What you've shown above isn't enough to say whether this is why they have both, but it's one possibility.

Comments

[JadeMason]

From what I can tell, this code never has a need for synchronization that crosses process boundaries. This leads me to believe that the CMutex in CCLassA could be replaced with a CCriticalSection.

[JadeMason]

Unfortunately, I'm stuck with MFC. My question was less about the difference between CMutex and CCriticalSection, and more about the difference between CSyncObject::Lock and CSingleLock::Lock. After doing some more research, it appears that CSingleLock behaves similarly to the boost::scoped_lock you show above (calls Unlock on destructor). I didn't realize CSingleLock prevented recursion, I assumed that each new instance on the stack would manage independently.

[AJG85]

CSyncObject calls the Windows api WaitForSingleObject under the covers as far as I know and can't be used directly. It is the base class interface that CMutex derives from so it's used when you do CMutex::Lock(); on your instance. CSingleLock is your CCriticalSection manipulator that is concrete and can be used as a scope lock with some wrapping. As mentioned before though there are some design problems with these objects so they should be avoided flounder.com/avoid_mfc_syncrhonization.htm

[JPhi1618]

For future readers, CSingleLock has a constructor parameter that will cause it to be locked on construction. The OP's example isn't using it, which sort of defeats the biggest reason to use a CSingleLock.