Re: Critical Section
On Nov 24, 9:50 pm, Joseph M. Newcomer <newco...@flounder.com> wrote:
Why is it so wrong to call Lock/Unlock? Well, Lock is not so bad, but
Unlock is catastrophic for exception-safety. Ask yourself this: are
you sure that there will be no exceptions between calls to Lock and
Unlock? And what will happen if there is? Answers are: no, you are
not. Even in simplest code like yours, there can be e.g.
CMemoryException (in InsertBusyThreadId). If there indeed is an
exception, your Unlock will not be called and your critical section
will stay locked, possibly forever.
****
Actually, this is a Good Thing, and the point I was making is that it is =
actually
extremely important that the critical section stay locked forever. Why=
? Because if you
throw an exception, YOU HAVE NO IDEA IF THE DATA BEING PROTECTED IS CORRE=
CT!
Fair enough, but that's just another thing to consider: code inside a
lock needs to have "strong" exception guarantee. E.g.
{
CSingleLock l(&syncObject, TRUE);
container1.push_back(data1);
ScopeGuard g1 = MakeObjGuard(container1, &c1type::pop_back);
container2.push_back(data2);
ScopeGuard g2 = MakeObjGuard(container2, &c2type::pop_back);
container3.push_back(data3);
// All fine, keep all data in
g2.Dismiss();
g1.Dismiss();
}
Of course, I presume here that any cleanup operations have no-throw
exception guarantee, which is reasonable and true relatively often.
(Ground rule of the scope guard is that the function it calls is a no-
throw one. It's really the same thing as the "destructors shalt not
throw" rule of C++).
The RAII
"feature" guarantees the lock is unlocked without actually guaranteeing t=
hat the data
integrity is preserved, which is really, really dangerous. Not to ment=
ion stupid. I'd
rather have the data stay locked forever than to be unlocked and broken, =
which is what the
CSingleLock mechanism promises.
Using lock/unlock guarantees that data is locked/unlocked without a
guarantee that data integrity is preserved just the same.
In the example above, if you use explicit lock/unlock and no scope
guard (or try/catch), and container2.push_back fails, you have utterly
horrible bug: the lock is there forever for any thread except the one
where container2.push_back failed, data is not consistent even in that
thread, and you can touch it later. In other words, CSingleLock or
explicit lock/unlock, you, the programmer, have to insure data
integrity. Both approaches are a 100% orthogonal to shared data
integrity.
Sure, RAII idiom, like all similar things of the sort, can lull you
into a false sense of security. But when used properly (e.g like
above), it's a good code-clarity help (in the example, the alternative
are repetitive try/catch-es, meh).
By the way, one of the most highly-regarded C++ libraries, boost, also
uses a variant of CSingleLock (they call it scoped_lock), and they
discourage explicit use of lock/unlock.
Goran.