Static CCriticalSection

[Tony M]

I have been trying to figure out if it is necessary to have a static
CCriticalSection in my class... Here is what I mean:

Suppose there will be many instances of class A wherein it has a
static member variable I need to protect. Do I need to use statics in
the CCriticalSection and/or CSingleLock?

class A : public CObject {
public:

INT GetI(){
//-----------------------------
//Like this????

CSingleLock singleLock(&m_CriticalSec_i, TRUE);
return m_i;

//-----------------------------
// Or Like This (with static CSingleLock...)

static CSingleLock* singleLock;
singleLock = new CSingleLock(&m_CriticalSec_i);
singleLock->Lock();
INT iReturnValue = m_i;
delete singleLock;
return iReturnValue;
}

void SetI(INT newI) {
...//similar stuff to above method....
}

private:
static INT m_i;
static CCriticalSection m_CriticalSec_i; //Does this need to be
static??
};

//Assuming there is external linkage for the statics here plus
// in the various calling code there are several instances in
different threads.

Thanks for any insight here!
TonyM ( lactaseman (at} yahoo {dot) com }

[David Lowndes]

> Suppose there will be many instances of class A wherein it has a
> static member variable I need to protect. Do I need to use statics in
> the CCriticalSection and/or CSingleLock?

If you have multiple threads, then you need a static CS to protect any
static data used by multiple threads.

> //Like this????
>...

> //-----------------------------
> // Or Like This (with static CSingleLock...)

Both code snippets are effectively the same - use the simpler.

Dave
--
MVP VC++ FAQ: http://www.mvps.org/vcfaq

[TonyM]

Thanks David,

I searched MSDN and the groups and could find a direct answer to this
although intuitively the staic CS seemed the correct action. Upon
looking into the code of the CSingleLock, it appears it just calls the
Lock/Unlock of the CS passed to it at construction time,.. so the
CSingleLock, I believe, does not need to be static, just the CS needsto
be static.

TonyM

[Joseph M. Newcomer]

Yes, you need a variable with the same scope and extent as the values you are protecting.
In this case, the critical section must be static, the same as the variable it is
protecting.

Avoid the use of 'new' unless you well and truly need a heap variable. In this case, a
pointer to a heap variable is silly, because if you allocate and delete in the same
function, why are you bothering to allocate at all?
joe

Joseph M. Newcomer [MVP]
email: newc...@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm

[Doug Harrison [MVP]]

Tony M wrote:

>I have been trying to figure out if it is necessary to have a static
>CCriticalSection in my class... Here is what I mean:
>
>Suppose there will be many instances of class A wherein it has a
>static member variable I need to protect. Do I need to use statics in
>the CCriticalSection and/or CSingleLock?
>
>class A : public CObject {
>public:
>
> INT GetI(){
> //-----------------------------
> //Like this????
>
> CSingleLock singleLock(&m_CriticalSec_i, TRUE);
> return m_i;

Yes. That code will acquire the mutex[*] when singleLock is initialized and
release it when singleLock goes out of scope.

> //-----------------------------
> // Or Like This (with static CSingleLock...)
>
> static CSingleLock* singleLock;
> singleLock = new CSingleLock(&m_CriticalSec_i);
> singleLock->Lock();
> INT iReturnValue = m_i;
> delete singleLock;
> return iReturnValue;
> }

This doesn't accomplish anything useful compared to the first example, and
it's actively harmful in a number of ways:

1. new/delete involving raw pointers introduce exception safety issues.
2. Related to (1), what happens if you issue an early return but forget to
delete the pointer?
3. There is more overhead for new/delete vs. a local variable, and the
static singleLock pointer takes up space in your data segment for no reason.
There's no reason for this pointer to be declared static.

In summary, there's no reason to use new/delete here vs. an auto variable as
in your first example.

> void SetI(INT newI) {
> ...//similar stuff to above method....
> }
>
>private:
> static INT m_i;
> static CCriticalSection m_CriticalSec_i; //Does this need to be
>static??

Because m_i is static, and you apparently need a mutex to protect m_i, yes,
m_CriticalSec_i also needs to be static. That way, all objects of type A
share the same instances of these two variables.

[*] When it doesn't matter, I refer to both CRITICAL_SECTION and the
kernel-level mutex object as "mutex", which is a lot less awkward to type
than CRITICAL_SECTION or CCriticalSection, which aren't very good names
anyway. The code sequence between the lock/unlock calls has traditionally
been called the "critical section", not the synchronization object itself.

--
Doug Harrison
Microsoft MVP - Visual C++

[Doug Harrison [MVP]]

TonyM wrote:

>Thanks David,
>
>I searched MSDN and the groups and could find a direct answer to this
>although intuitively the staic CS seemed the correct action. Upon
>looking into the code of the CSingleLock, it appears it just calls the
>Lock/Unlock of the CS passed to it at construction time,.. so the
>CSingleLock, I believe, does not need to be static, just the CS needsto
>be static.

Correct. The CSingleLock type is intended to be used to declare auto
variables and is an expression of the RAII pattern. I can't think of a case
in which a static duration CSingleLock makes sense.

[TonyM]

Wow...! Yes to all your points! Thank you very much for the clarity
of thought and help.

Many Thanks,
TonyM