is Ccriticalsection usable in production?

Question

We are a couple of newbies in MFC and we are building a multi-threded application. We come across the article in the URL that warns us not to use CCriticalSection since its implementation is broken. We are interested to know if anyone has any experience in using CCriticalSection and do you come across any problems or bugs? Is CCriticalSection usable and production ready if we use VC++ 2008 to build our application?

http://www.flounder.com/avoid_mfc_syncrhonization.htm

thx

Answer 1

I think that article is based on a fundamental misunderstanding of what CSingleLock is for and how to use it.

You cannot lock the same CSingleLock multiple times, but you are not supposed to. CSingleLock, as its name suggests, is for locking something ONCE.

Each CSingleLock just manages one lock on some other object (e.g. a CCriticalSection which you pass it during construction), with the aim of automatically releasing that lock when the CSingleLock goes out of scope.

If you want to lock the underlying object multiple times you would use multiple CSingleLocks; you would not use a single CSingleLock and try to lock it multiple times.

Wrong (his example):

CCriticalSection crit;
CSingleLock lock(&crit);
lock.Lock();
lock.Lock();
lock.Unlock();
lock.Unlock();

Right:

CCriticalSection crit;
CSingleLock lock1(&crit);
CSingleLock lock2(&crit);
lock1.Lock();
lock2.Lock();
lock2.Unlock();
lock1.Unlock();

Even better (so you get RAII):

CCriticalSection crit;
// Scope the objects
{
    CSingleLock lock1(&crit, TRUE); // TRUE means it (tries to) locks immediately.
    // Do stuff which needs the lock (if IsLocked returns success)
    CSingleLock lock2(&crit, TRUE);
    // Do stuff which needs the lock (if IsLocked returns success)
}
// crit is unlocked now.

(Of course, you would never intentionally get two locks on the same underlying critical section in a single block like that. That'd usually only happen as a result of calling functions which get a lock while inside something else that already has its own lock.)

(Also, you should check CSingleLock.IsLocked to see if the lock was successful. I've left those checks out for brevity, and because they were left out of the original example.)

If CCriticalSection itself suffers from the same problem then that certainly is a problem, but he's presented no evidence of that that I can see. (Maybe I missed something. I can't find the source to CCriticalSection in my MFC install to verify that way, either.)

Answer 2

That article suggests that a simple situation of using those primitives are fine, except that the implementation of them violates the semantics that should be expected of them.

basically, it suggests that if you use it as a non-recursive lock, where you take care to always ensure that the lock is valid (ie, not abandoned), then you should be fine.

The article does complain, however, that the limitations are inexcusable.