Where to initialize CRITICAL_SECTION in a singleton?

[Salvadoravi]

Hi all,

I want to use a CRITICAL_SECTION in my singleton class to synchronize access by multiple threads.

I need to lock the singleton even after initialization so the double checked lock perils are not an issue for me.

What I don't unserstand is where do I need to initialize my critical section?

Can someone show me in the following code where InitilalizeCriticalSection() needs to be used?

class CSingleton
{
private:
    static CRITICAL_SECTION m_cs;
    static CSingleton *pInstance;

    CSingleton();
    CSingleton(const CSingleton&);
    CSingleton&  operator=(const CSingleton&);
    
public:  
    static CSingleton& Instance(); 
};

Singleton* CSingleton::pInstance = NULL;
Singleton* CSingleton::m_cs = 0;

Singleton* Singleton::Instance()
{
    EnterCriticalSection(&m_cs); // To simplify I'm not using raii guard
	
    if ( !pInstance )
        pInstance = new CSingleton;
  
    // Do stuff............

    LeaveCriticalSection(&m_cs);
    return *pInstance;
}

Many thanks!!!!

[ovidiucucu]

At a first try, you can do it as follows:

// ...
CRITICAL_SECTION CSingleton::m_cs = {0};

CSingleton& CSingleton::Instance()
{
   if(NULL == m_cs.DebugInfo)
   {
      InitializeCriticalSection(&m_cs);
   }
   // the rest of stuff...

This is not a very brilliant approach (I can imagine next question(s): where to better call DeleteCriticalSection and what about two threads "simultaneously" check for m_cs.DebugInfo value?).
So, a little bit better is to define your own class which encapsulates the critical section functionality:

class CriticalSection
{
   CRITICAL_SECTION m_cs;
public:
   CriticalSection() 
   {
      ::InitializeCriticalSection(&m_cs);
   }
   ~CriticalSection() 
   {
      ::DeleteCriticalSection(&m_cs);
   }
   void Lock()
   {
      ::EnterCriticalSection(&m_cs);
   }
   void Unlock()
   {
      ::LeaveCriticalSection(&m_cs);
   }
};

class CSingleton
{
   static CriticalSection m_cs;
   // ...
};

CriticalSection CSingleton::m_cs;
CSingleton& CSingleton::Instance()
{
   m_cs.Lock();

   if (!pInstance)
   {
      pInstance = new CSingleton;
   }

   m_cs.Unlock();
   return *m_pInstance;
}

Or, if you are using MFC, you can use the already built class CCriticalSection.

[Salvadoravi]

Thank you very much fo your input,

At a first try, you can do it as follows:

// ...
CRITICAL_SECTION CSingleton::m_cs = {0};

CSingleton& CSingleton::Instance()
{
   if(NULL == m_cs.DebugInfo)
   {
      InitializeCriticalSection(&m_cs);
   }
   // the rest of stuff...

[/code]

Initializing a critical section that has already been initialized results in undefined behavior.
The code is not thread safe.

So, a little bit better is to define your own class which encapsulates the critical section

Agree, encapsulating a critical section is generally a good idea but doing so will result that the critical section initializtion occur at run time which will introduce the non local static objects initialzation fiasco (objects may use getInstance() before the critical section has been initialized).

This basically mean that implemeting it like this will restrict the users of the library (and myself) not to use the singleton class in classes that use the singleton in their constructor, if these classes can be global, static, etc...(all classes 8).

Because my singleton is a logger class that I want to use in all of my classes, and these classes may be static global, etc, this solution is not what I'm after.

Or, if you are using MFC, you can use the already built class CCriticalSection.

I'm not using MFC

Thoughts?
Many thanks!!

[ovidiucucu]

[...]

[...] this solution is not what I'm after.[...]

Thoughts?
[...]

Now it looks like a quiz...

Well then, take a look in chapter 6 of Modern C++ Design by Andrei Alexandrescu.
Hopefully you'll find an answer for the noisy problem of generalizing singletons.

[Salvadoravi]

Now it looks like a quiz...

Well then, take a look in chapter 6 of Modern C++ Design by Andrei Alexandrescu.
Hopefully you'll find an answer for the noisy problem of generalizing singletons.

Thank you for your suggestion, I have the book but unfortunaltely The implemention of the mutex in the book is opaque.


Thanks again!!

[originalfamousjohnsmith]

Or, how about no critical section at all? Just because you make a singleton doesn't mean it has to have deferred creation. That is usually a forward optimization, and one responsible for many bugs and lots of performance bottlenecks.

You can just make a static member instance of the class that gets initialized statically and optionally make the methods all static.

[OReubens]

Regardless of any solution you take, there is ALWAYS at least one critical section that needs to be created/initialized BEFORE any threads are allowed to run.

The easy way out for a singleton class is to initialise the critical section for that singleton at startup of the runtime, which is what would happen if you include your singleton header file which has that 'static CCriticalSection m_cs;' line that constructs and initialised the critical section even before main() (or WinMain) gets called. The solution presented by ovidiucucu handles it that way.

The problem is of course... if you have a library with potentially hundreds of singleton classes, you'd be initialising a lot of critical sections from the start. This isn't normally an issue, but if it is, you can create 1 librarywide critical section, which will safeguard the creation of class specific critical sections/mutexes/semaphores/events. Done properly it's not a performance issue since each class would only need to lock this librarywide critical section exactly once. Done wrong you'd end up locking your library CS on each instancing which would be a serious performance issue.

[Salvadoravi]

Thanks again for your inputs

You can just make a static member instance of the class that gets initialized statically and optionally make the methods all static.

You suggestion is the implemention of the mono state pattern not a singleton.

if you include your singleton header file which has that 'static CCriticalSection m_cs;' line that constructs and initialised the critical section

To initialize a critical section you need to call InitializeCriticalSection(CRITICAL_SECTION).
This will take place at runetime.

The problem is of course... if you have a library with potentially hundreds of singleton classes, you'd be initialising a lot of critical sections from the start. This isn't normally an issue

Again, Initializing a critical section that has already been initialized results in undefined behavior.


Gurus?

[ovidiucucu]

To initialize a critical section you need to call InitializeCriticalSection(CRITICAL_SECTION).
This will take place at runetime.

Isn't it OK? Or maybe it should take place before Adam & Eve creation...

[OReubens]

that's what you want no ? The constructor of the CCriticalSection class would call the initialise the structure. Becauyse the CCriticalSection is static, the constructor gets called (at runtime) at the very start of the program.

There is _NO_ way to have a critical section initialised before your code is even running, so it will ALWAYS be initialised somewhere at runtime.

[Igor Vartanov]

Agree, encapsulating a critical section is generally a good idea but doing so will result that the critical section initializtion occur at run time which will introduce the non local static objects initialzation fiasco (objects may use getInstance() before the critical section has been initialized).

Static an global variables are constructed long before any calls become functional on the loaded code. So your fiasco's possibility is imaginary only.

[Salvadoravi]

Static an global variables are constructed long before any calls become functional on the loaded code. So your fiasco's possibility is imaginary only.

"The problem we're concerened with, involves at least to separately compiled source files, each of which contains at least one non local static object (i.e an object that's global, namespace scope or static in a class or at file scope). And the actual problem is this : if initialzation of a non-local static object in one translation unit uses a non-local static object in a different translation unit, the object it uses could be uninitialized." [Meyers]

[Igor Vartanov]

So, this must give you a hint: you'd better place your critical section initialization into a constructor of the class that implements your singletone.

[originalfamousjohnsmith]

Thanks again for your inputs



You suggestion is the implemention of the mono state pattern not a singleton.

Singleton can be monostate but still singleton. As I already said, they are separate issues.

I won't say they should never be used like you are wanting to but it's almost never what people really need so if you don't need it don't do it.