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?

To add some context, one of my concerns is if the use of CSingleLock::Lock has subtly different behavior. For instance, where the same thread might be able to call CMutex::Lock multiple times (since it already owns the lock), a call to CSingleLock::Lock on the same instance of CSingleLock will block. I'm also concerned that I may run into a situation where a CSingleLock is managing a CCriticalSection, but that CCriticalSection has the Unlock method called directly. — JadeMason, May 19 '11 at 18:41

score 2 · Answer 1 · answered May 19 '11 at 18:27

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.

answered May 19 '11 at 18:27

Jerry Coffin

476,176
80
629
1,111

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 May 19 '11 at 18:31

score 2 · Accepted Answer · answered May 23 '11 at 17:54

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.

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. — JadeMason, May 23 '11 at 20:21
`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 http://www.flounder.com/avoid_mfc_syncrhonization.htm — AJG85, May 23 '11 at 20:29
For future readers, CSingleLock has a [constructor](https://msdn.microsoft.com/en-us/library/fw63hszf.aspx) 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. — JPhi1618, Feb 16 '16 at 21:53

CMutex::Lock vs. CSingleLock::Lock

2 Answers2