[SOLVED] How can I make a Class be instantiated only ONCE?
-
I also recommend to not use a static member.
You will have to use a static method local instead of a static member in order to avoid "initialization order problems":http://www.parashift.com/c++-faq/static-init-order.html, which will otherwise lead to undefined behaviour (mind QCoreApplication).
In addition, you will have to use a static method local instead of a static member in order to lazy-initialize your instance, which is an absolute requirement in Qt to access a large part of QtWidgets and its graphics stack, as they depend on an existing QApplication instance (which is not true at static initialization time).
A static member will also increase your application startup time.
QtWidgets itself is inherently not thread-safe; if you still need thread-safe initialization for your class follow "Q_GLOBAL_STATIC":http://code.woboq.org/qt5/qtbase/src/corelib/global/qglobal.h.html#738 (this is an MSVC issue only, as static method local initialization is always thread-safe in GCC and Clang, be it C++98 or C++11).
And yes, singletons should be considered an anti-pattern, as they violate the single responsibility principle. If you want to make sure that there is just a single instance make the constructor private and friend a factory, which allows only one instance to be created (as in <code>meyersSingleton</code>).
-
[quote author="Lukas Geyer" date="1357810573"]I also recommend to not use a static member.
You will have to use a static method local instead of a static member in order to avoid "initialization order problems":http://www.parashift.com/c++-faq/static-init-order.html, which otherwise will lead to undefined behaviour (mind QCoreApplication).[/quote]
That would be a problem if we made the static member variable an (auto) object. But if we make it a pointer to an object (statically initialized to NULL), as was suggested above, there should be no such problem, right? The object will be created at the moment when getInstance() is called for the very first time, so behavior is well-defined. And if we use a Mutex in that function, as shown in the article linked by vidar, it will even be thread save :-)
Also: If we made the variable an (auto) object rather than a pointer and declare it as a local static variable, the resulting code would absolutely not be thread save! That's because this code:
@void MyClass* MyClass::getInstance(void)
{
static MyClass instance;
return &instance;
}@Would be expanded by the compiler to something (analogously) like:
@void MyClass* MyClass::getInstance(void)
{
static _init = false;
static MyClass instance;if(!_init) { &instance = new MyClass(); /*construct on first call*/ _init = true; } return &instance;}@
Now it should be easy to spot the race-condition... ("more":http://blogs.msdn.com/b/oldnewthing/archive/2004/03/08/85901.aspx)
--
Only problem I see with the static member variable solution is that the object never gets destroyed - unless we explicitly add a static MyClass:destroyInstance() method and call it at the end of our main() function. Either that or we add some atexit() magic to MyClass:getInstance() to do the clean-up automatically on termination:
@MyClass *MyClass::m_instance = NULL;
MyClass* MyClass::getInstance(void)
{
QMutexLocker lock(&m_mutex);
if(m_instance == NULL)
{
m_instance = new MyClass();
atexit(doCleanup);
}
return m_instance;
}void MyClass::doCleanup(void)
{
QMutexLocker lock(&m_mutex);
if(m_instance != NULL)
{
delete m_instance;
m_instance = NULL;
}
}@ -
[quote author="MuldeR" date="1357818537"]That would be a problem if we made the static member variable an (auto) object. But if we make it a pointer to an object (statically initialized to NULL), as was suggested above, there should be no such problem, right?[/quote]-Yes.
[quote author="MuldeR" date="1357818537"]Also: If we made the variable an (auto) object rather than a pointer and declare it as a local static variable, the resulting code would absolutely not be thread save![/quote]Absolutely. But as said, QtWidgets is not thread-safe at all as well, so the requirement for thread-safety is unlikely, whereas the requirement for defined behaviour and lazy initialization is absolute (which will require the use of a static method local object instead of a static member object).
You will have to add thread-safety when needed (either using a mutex, which is quite costly, or better an atomic test-and-set-ordered instruction as seen in Q_GLOBAL_STATIC, and in case your compiler does not support thead-safe static method local object initialization anyway).
[quote author="MuldeR" date="1357818537"]Only problem I see with the static member variable solution is that the object never gets destroyed - unless we explicitly add a static MyClass:destroyInstance() method and call it at the end of our main() function. Either that or we add some atexit() magic to MyClass:getInstance() to do the clean-up automatically on termination.[/quote]A common solution is to use a thread-safe wrapper instead of the pointer itself, which deletes the object during its destruction.
@
MyClass* MyClass::getInstance(void)
{
QMutexLocker lock(&m_mutex);
if(m_instance == NULL)
{
m_instance = new MyClass();
atexit(doCleanup);
}
return m_instance;
}
@
Be aware that this code already yields undefined behaviour. Due to <code>m_mutex</code> beeing a static member it has to be initialized using <code>QMutex MyClass::m_mutex</code> at global scope, whose initialization order is undefined. If there is another static member which calls <code>MyClass::getInstance()</code> directly or indirectly you are at a 50% chance of using an uninitialized QMutex. You will have to use static method local instead of the static member (<code>QMutex &m_mutex() { static QMutex mutex; return mutex; }</code> be aware that there is no thread-safety required here, as static initialization is always done in a single-threaded context). -
I'm not aware of a better method than making those "global" Mutex'es (auto) objects. We cannot initialize them "lazy", because to initialize a Mutex the "lazy" way at runtime, we'd need another Mutex for protection - leading to an infinite sequence of Mutex'es. We could do a globalInit() early in main(), but that's pretty much the same.
However it should be save to have a global QMutex as (auto) object, because the Mutex will be initialized before the main() function even enters and thus before anybody calls getInstance(). The order doesn't really matter, because all we need is that all global Mutex'es have been initialized when the main() function starts.
Now if we have another global (auto) object, which inside its constructor calls getInstance(), then that's simply a programming mistake. Constructors of global objects really should not try to access other objects! :-O
--
To harden our code against this kind of programming mistake, maybe we could do:
@static QMutex *g_mutex = NULL;
static volatile long g_init = 0L;void globalInit(void)
{
if(_InterlockedCompareExchange(&g_init, -1L, 0L) != 0L)
{
/fatal error: must be called exactly once!/
abort();
}g_mutex = new QMutex(); /*more initialization work*/_InterlockedCompareExchange(&g_init, 1L, -1L);
}MyClass *getInstance(void)
{
if((g_init != 1L) || !g_mutex)
{
/fatal error: not initialized yet!/
abort();
}QMutexLocker(g_mutex); /* ... */}
// ------------
int main(...)
{
globalInit();/* ... */ MyClass *ptr = getInstance(); /* ... */}@
-
You will have to differentiate between static initialization order safety and thread safety.
Static initialization order safety needs not to be thread safe, as there is just a single thread at the time static objects are initialized (before main); you need no mutex. You just need to make sure that an object exists at the time it is accessed.
[quote author="MuldeR" date="1357825548"]Now if we have another global (auto) object, which inside its constructor calls getInstance(), then that's simply a programming mistake. Constructors of global objects really should not try to access other objects![/quote]No, it isn't and yes, they should if they need to. And you will have to beware of it when creating code that is subject to static initialization order (as for instance an accessible static member or a static global).And even if you apply this rule to your local coding guidelines - since when has forbidding making programming mistakes stopped developers doing so? ;-)
Please be aware that I've updated the first answer of the previous post, as I have completely misunderstood your question.
-
[quote author="Lukas Geyer" date="1357824375"]No, your pointer will still be subject to static initialization order problems (the pointer might be read before it has been initialized with NULL), but not to the lazy initialization problem.[/quote]
Just wanted to add that this problem does not exist in practice. The global pointer variable does not actually need to be initialized "actively" - unlike Object's whose constructor needs to be called. The linker will simply assign that variable an address within the executables 'data' section and at this address the correct initial value (NULL in this case) is stored. So the pointer will be NULL already at the moment when the executable's image is loaded into memory by the os's loader... long before any static initializers (in whatever order) are executed.
(I don't know if the C standard really guarantees that behavior. It probably does but I haven't checked. Anyway, it's at least what all practical compilers will do)
There even exist a special section (BSS) for zero-initialized variables:
http://en.wikipedia.org/wiki/.bss--
[quote author="Lukas Geyer" date="1357824375"]And even if you apply this rule to your local coding guidelines – since when has forbidding making programming mistakes stopped developers doing so? ;-)[/quote]
Well, with the global initialization function (see previous post) or some similar approach we could enforce the correct use. It adds some more complexity though...
-
[quote author="MuldeR" date="1357832140"]Just wanted to add that this problem does not exist in practice.[/quote]No, it doesn't. I stand corrected, the initial answer was correct (I somehow had non-exclipt initalized in mind).
[quote]Objects with static storage duration (3.7.1) shall be zero-initialized (8.5) before any other initialization takes place. Zero-initialization and initialization with a constant expression are collectively called static initialization; all other initialization is dynamic initialization. Objects of POD [plain old data] types (3.9) with static storage duration initialized with constant expressions (5.19) shall be initialized before any dynamic initialization takes place. Objects with static storage duration defined in namespace scope in the same translation unit and dynamically initialized shall be initialized in the order in which their definition appears in the translation unit.[/quote]
[quote author="MuldeR" date="1357832140"]Well, with the global initialization function (see previous post) or some similar approach we could enforce the correct use. It adds some more complexity though...[/quote]Or just create static initialization order aware code, so it is in fact a non-issue. ;-) -
[quote author="Lukas Geyer" date="1357835466"][quote author="MuldeR" date="1357832140"]Just wanted to add that this problem does not exist in practice.[/quote]No, it doesn't. I stand corrected, the initial answer was correct.
[quote]Objects with static storage duration (3.7.1) shall be zero-initialized (8.5) before any other initialization takes place. Zero-initialization and initialization with a constant expression are collectively called static initialization; all other initialization is dynamic initialization. Objects of POD [plain old data] types (3.9) with static storage duration initialized with constant expressions (5.19) shall be initialized before any dynamic initialization takes place. Objects with static storage duration defined in namespace scope in the same translation unit and dynamically initialized shall be initialized in the order in which their definition appears in the translation unit.[/quote][/quote]???
First you said there may be a problem in my getInstance() method because the pointer (global and static) may not have been initialized to NULL (zero) yet when that function is called. I then tried to explain why that problem doesn't exist in practice. Now you show an excerpt from the standard which proves exactly my point. But if I understand your post correctly, you still think there is a problem?
Your excerpt clearly says: Objects of POD [plain old data] types with static storage duration initialized with constant expressions shall be initialized before any dynamic initialization takes place. According to that, the "instance" pointer gets initialized to NULL before any global Object is constructed (as the latter would be a "dynamic initialization") - and therefore before anything might potentially try to call the getInstance() method. The only thing that might potentially call getInstance() even before the main() function is entered would be a "dynamic" initializer, e.g. a constructor of a global object. And this can happen only after static initialization has been done.
-
[quote author="MuldeR" date="1357840096"]Yup. Sorry for the confusion...[/quote]No sweat.
On a sidenote: the "implementation":http://www.qtcentre.org/wiki/index.php?title=Singleton_pattern linked by vidar is not thread safe either, because it is prone to "instruction reordering issues":http://erdani.com/publications/DDJ_Jul_Aug_2004_revised.pdf (<code>m_Instance = new Singleton</code> is not an atomic operation, and the thread may get suspended before <code>Singleton</code> is constructed, but after <code>m_Instance</code> has been allocated and assigned).
-
Seems like this thread became interesting :)
As far as my goal is concerned (and being just a mid-level dev), the "Singleton implementation":http://www.qtcentre.org/wiki/index.php?title=Singleton_pattern works fine on me.
Thank you guys for the help! I will mark this thread [SOLVED] but you may still continue the discussion (and I will spend some time reading your comments since I think it is really interesting).