Re: CSingleLock - known behaviour?
On Jun 26, 12:40 pm, neilsolent <n...@solenttechnology.co.uk> wrote:
Joseph M. Newcomer wrote:
I would have done it as my equivalent of
void Function1()
{
CSingleLock mylock(&lock);
mylock.Lock();
if(condition1)
...
if(condition2)
...
But now you are locking the CS unnecessarily (if condition1 and
condition2 are both FALSE).
Do it this way instead, which (I think) emphasizes the two points that
Doug Harrison has been trying to make: RAII is the better paradigm,
and don't call CSingleLock::Lock more than a single time:
void Function1()
{
if (condition1)
{
CSingleLock lk(&m_cs, TRUE);
/* do stuff for condition 1 */
}
if (condition2)
{
CSingleLock lk(&m_cs, TRUE);
/* do stuff for condition 2 */
}
/* etc for N times */
if (conditionN)
{
CSingleLock lk(&m_cs, TRUE);
/* do stuff for condition N */
}
}
Note that although there are N CSingleLock objects all named "lk", all
of them are different, and each is an automatic variable whose
lifetime is precisely the set of enclosing braces from the "if"
statement. Because of this limited lifetime, you get RAII. Because
each CSingleLock object is different, you call Lock() on each only
exactly once.
And, per your concern about locking the CS "unnecessarily", the above
code doesn't. However, I also agree with Doug that this might not be
a true concern. It's true in general that you should hold a lock on
shared data for as brief a period as possible, to promote efficient
multi-threading, but unless all of your conditions are extensive, it's
probably clearer to hold the lock for the entirety of "Function1", as
suggested by JMN. Stated another way, it's hard to see the benefit of
locking and unlocking frequently, at the expense of obscuring the
purpose of the code.