Singletons and destructors
Rune Allnor
I have struggled for a couple of days with a singleton pattern.
It turned out that the cause of the problem is that the destructor
of a singleton pattern is not called unless somebody calls a
'delete' to a pointer to the singleton.
Below is an example, where the program reports what methods
it calls. The only way I can find to see a report from the
singleton::destructor is to uncomment the 'delete' statement
indicated.
This is a bit awkward, since there may be more than one
refernece to the singleton, and it is not at all clear which
refernce should be burdened with the responsibility to
delet the singleton.
The obvious solution is to re-write the singleton pattern in
terms of smart pointers. I don't want to reinvent the wheel,
so is there a standard singleton pattern out there which
uses smart pointers?
Rune
///////////////////////////////////////////////////////////////
//#include "stdafx.h" // Uncomment if you use visual studio
#include <iostream>
class singleton{
public:
static singleton* instance();
~singleton();
protected:
singleton();
private:
static singleton* instance_;
};
singleton* singleton::instance_=0;
singleton::singleton()
{
std::cout << "In constructor" << std::endl;
}
singleton::~singleton()
{
std::cout << "In destructor" << std::endl;
}
singleton* singleton::instance()
{
std::cout << "In 'instance'" << std::endl;
if (instance_ == 0)
{
instance_ = new singleton;
}
return instance_;
}
int main(int argc,char* argv[])
{
singleton* s = singleton::instance();
singleton* t = singleton::instance();
// delete t; // <<< === Uncomment to call destructor
return 0;
}
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Hrayr BABAJANYAN
Hello,
Well if you use the implementation of the singleton you provided the
deletion of the instance can be made using the std::atexit.
#include <cstdlib> // std::atexit
class Singleton
{
public:
static Singleton* instance()
{
if (instance_ = 0)
instance_ = new Singleton();
return instance_;
}
static void free()
{
delete instance_;
}
protected:
Singleton();
~Singleton();
private:
static Singleton* instance_;
};
Singleton Singleton::instance_ = 0;
// ...
int main()
{
std::atexit(Singleton::free);
// ...
return 0;
}
And the implementation that I prefer is the following (here you don't
need to care about deletion of the instance - it will be deleted at
the program exit (but you also will not have a control on the deletion
of the instance).
class Singleton
{
public:
static Singleton& instance()
{
static Singleton anInstance; // the ctor is being called only once
(initialization of local static variable)
return anInstance;
}
protected:
Singleton();
~Singleton();
};
PS: you may be interested in more detailed discussion of this (well
known problem), so I would refer books by Alexandresku, Mayers and
Sutter (unfortunately I don't have them with me right now so I cannot
tell you the chapters).
Kind regards,
--
Hrayr
aman.c++
> Hi all.
>
> I have struggled for a couple of days with a singleton pattern.
> It turned out that the cause of the problem is that the destructor
> of a singleton pattern is not called unless somebody calls a
> 'delete' to a pointer to the singleton.
>
> Below is an example, where the program reports what methods
> it calls. The only way I can find to see a report from the
> singleton::destructor is to uncomment the 'delete' statement
> indicated.
>
> This is a bit awkward, since there may be more than one
> refernece to the singleton, and it is not at all clear which
> refernce should be burdened with the responsibility to
> delet the singleton.
>
> The obvious solution is to re-write the singleton pattern in
> terms of smart pointers. I don't want to reinvent the wheel,
> so is there a standard singleton pattern out there which
> uses smart pointers?
>
> Rune
Rune,
There's no one-size-fits-all pattern and making singleton destruction
generic is especially non-trivial.
Here are a few basic tips about singleton destruction.
Think about the LIFETIME of your singleton. In many cases singletons
persist for lifetime of the program. In those cases you need to think
what needs to be cleaned up at program exit.
In the most trivial implementation where your singleton persists for
the lifetime of program and is just consuming memory as a resource,
you might be freed from the trouble of destroying it. In other cases
you might need some cleanup/destruction at program exit or during
program execution.
But the destruction should be responsibility of the singleton class
(just as creation is).
Clients of the singleton shouldn't ideally be deleting it.
regards,
Aman Angrish
Rune Allnor
> { Please don't quote the clc++m banner, removed. -mod }
>
> On Jul 23, 8:23 pm, Rune Allnor <all...@tele.ntnu.no> wrote:
> > I have struggled for a couple of days with a singleton pattern.
> > It turned out that the cause of the problem is that the destructor
> > of a singleton pattern is not called unless somebody calls a
> > 'delete' to a pointer to the singleton.
..
> PS: you may be interested in more detailed discussion of this (well
> known problem), so I would refer books by Alexandresku, Mayers and
> Sutter (unfortunately I don't have them with me right now so I cannot
> tell you the chapters).
If would be very useful if somebody could come up with specific
references. I've found some mentioning in Alxandrescu's "Modern
C++ design".
Rune
Greg Herlihy
> Below is an example, where the program reports what methods
> it calls. The only way I can find to see a report from the
> singleton::destructor is to uncomment the 'delete' statement
> indicated.
>
> This is a bit awkward, since there may be more than one
> refernece to the singleton, and it is not at all clear which
> refernce should be burdened with the responsibility to
>
> The obvious solution is to re-write the singleton pattern in
> terms of smart pointers.
Why call "new" to allocate the singleton in the first place? Wouldn't
the more obvious solution be to avoid "new" and "delete" by having the
singleton be statically - instead of dynamically - allocated? In fact,
the "classic" singleton pattern takes such an approach:
#include <iostream>
class singleton
{
public:
static singleton* instance();
~singleton();
protected:
singleton();
};
singleton::singleton()
{
std::cout << "In constructor" << std::endl;
}
singleton::~singleton()
{
std::cout << "In destructor" << std::endl;
}
singleton* singleton::instance()
{
static singleton instance_;
std::cout << "In 'instance'\n";
return &instance_;
}
int main(int argc,char* argv[])
{
singleton* s = singleton::instance();
singleton* t = singleton::instance();
// no delete singleton call
}
Program Output:
In constructor
In 'instance'
In 'instance'
In destructor
Greg
Joshua...@gmail.com
> Why call "new" to allocate the singleton in the first place? Wouldn't
> the more obvious solution be to avoid "new" and "delete" by having the
> singleton be statically - instead of dynamically - allocated? In fact,
> the "classic" singleton pattern takes such an approach:
I'd suggest seeing the FAQ, and the static deinitialization order
fiasco. As others have said in this thread, it's not a simple problem
with a simple solution, especially in multithreaded environments.
Greg Herlihy
> On Jul 24, 12:58 pm, Greg Herlihy <gre...@mac.com> wrote:
>
> > Why call "new" to allocate the singleton in the first place? Wouldn't
> > the more obvious solution be to avoid "new" and "delete" by having the
> > singleton be statically - instead of dynamically - allocated? In fact,
> > the "classic" singleton pattern takes such an approach:
>
> I'd suggest seeing the FAQ, and the static deinitialization order
> fiasco. As others have said in this thread, it's not a simple problem
> with a simple solution, especially in multithreaded environments.
Anyone who follows your suggestion and reads the C++ FAQ - will learn
that there is no such thing as the "static deinitialization order
fiasco" in C++ (at least as far as the FAQ is concerned). After all,
the destruction of static objects is well-defined (static objects are
destroyed in the reverse order of their construction.)
Perhaps you are referring to what the FAQ calls C++'s "static -
initialization- order" fiasco (in which the order that statically-
allocated objects are initialized - is not always specified in C++).
However, there is no such problem with the timing of the singleton's
initialization in the sample program I posted. The singleton in that
program is certain to be initialized the very first time instance() is
called - which is exactly the same point of initialization as in the
original program.
There are other ways to show that the sample program I posted is
sound. Logically, if it is safe for the program to delete the
singleton object before main() exits, then it must also be safe to
delete that same singleton, at a later point - namely, after main()
exits. The danger is deleting an object too early, there is no danger
in deleting an object "too late". Furthermore, in a multithreaded
environment, deleting the singleton object after main() exits (that
is, after all the other threads have terminated), clearly makes the
most sense.
Of course all of these points are purely academic - I could have
simply pointed out that the singleton object is the only statically-
allocated object in the program I posted. Whereas - a program would
have to declare two or more statically-allocated objects - before it
could even begin to have a problem with the order in which they are
initialized.
Greg
Joshua...@gmail.com
> On Jul 24, 3:10 pm, JoshuaMaur...@gmail.com wrote:
>
> > On Jul 24, 12:58 pm, Greg Herlihy <gre...@mac.com> wrote:
>
> > > Why call "new" to allocate the singleton in the first place? Wouldn't
> > > the more obvious solution be to avoid "new" and "delete" by having the
> > > singleton be statically - instead of dynamically - allocated? In fact,
> > > the "classic" singleton pattern takes such an approach:
>
> > I'd suggest seeing the FAQ, and the static deinitialization order
> > fiasco. As others have said in this thread, it's not a simple problem
> > with a simple solution, especially in multithreaded environments.
>
> Anyone who follows your suggestion and reads the C++ FAQ - will learn
> that there is no such thing as the "static deinitialization order
> fiasco" in C++ (at least as far as the FAQ is concerned). After all,
> the destruction of static objects is well-defined (static objects are
> destroyed in the reverse order of their construction.)
http://www.parashift.com/c++-faq-lite/ctors.html
[10.14] Why doesn't the construct-on-first-use idiom use a static
object instead of a static pointer?
Paraphrasing:
The point is simple: if there are any other static objects whose
destructors might use the singleton ans after ans is destructed, bang,
you're dead. If the constructors of other singletons a, b and c use
ans, you should normally be okay since the runtime system will, during
static deinitialization, destruct ans after the last of those three
objects is destructed. However if a and/or b and/or c fail to use ans
in their constructors and/or if any code anywhere gets the address of
ans and hands it to some other static object, all bets are off and you
have to be very, very careful.
Rune Allnor
> On Jul 23, 6:23 pm, Rune Allnor <all...@tele.ntnu.no> wrote:
>
> > Below is an example, where the program reports what methods
> > it calls. The only way I can find to see a report from the
> > singleton::destructor is to uncomment the 'delete' statement
> > indicated.
>
> > This is a bit awkward, since there may be more than one
> > refernece to the singleton, and it is not at all clear which
> > refernce should be burdened with the responsibility to
> > delete the singleton.
>
> > The obvious solution is to re-write the singleton pattern in
> > terms of smart pointers.
>
> Why call "new" to allocate the singleton in the first place? Wouldn't
> the more obvious solution be to avoid "new" and "delete" by having the
> singleton be statically - instead of dynamically - allocated?
If it can be done - of course. How does the code you posted work?
I can find only two declarations of 'static' in the whole code,
the method singleton::instance() and the local variable inside
singleton::instance(). Are those declarations enough to force
the whole class go static? Please humor me as I 'think out
loud' through the code:
If I understand things correctly, the local variable
singleton::instance_ inside singleton::instance() will
be allocated at the first call to singleton::instance().
The address of this variable is returned at first call.
Since the variable is declared 'static' it will remain
in global memory until program termination, when its
destructor will automatically be called, since no
call to 'new' was used.
Ah, since singleton::instance() is 'static' it is the same
function handle/address/memory footprint/whatever the correct
term might be, which is called every time, ensuring that
it is the same local static copy of instance_ which is
accessed every time... hey, all of a sudden the code starts
to make sense!
[BTW, now would be a good time for somebody to correct me if
my understanding of this scheme is flawed in any way...]
Is there a reference to this 'classic' singleton pattern
somewhere? I based my suggestion on the recipe in Gamma & al.
Rune
Jiang
> Is there a reference to this 'classic' singleton pattern
> somewhere? I based my suggestion on the recipe in Gamma & al.
>
For the above "Meyers-Singleton", please read "More Effective C++"
by Scott Meyers. Sure you've read it, right? :-)
Singleton is a special pattern, here "special" means, it is quite easy
to understand how it works at the first glance, but is extremely
difficult to make it really work. Yes, as other said, there is no such
a silver-bullet can solve all the problems.
Please have a look at Loki::Singleton at
http://loki-lib.svn.sourceforge.net/viewvc/loki-lib/trunk/include/loki/Singleton.h?view=markup
, maybe it is not the singleton as you expected. :-)
So my suggestion is, well, check your targets carefully and find
the necessary trade-offs to make the life easier. For example,
* If you can make sure you will not do mulit-threading
in your program, then lock issues can be saved.
* If you can change your definition of "memory leak",
then you do not have to delete the singleton in the heap,
since most of the OS will handle this issue for you.
(But this does not apply to other resource, such as
GUI handles, .etc)
P.S. Please make sure you really need the Singleton,
someone believes Singleton is an Anti-Pattern.
http://blogs.msdn.com/scottdensmore/archive/2004/05/25/140827.aspx
http://tech.puredanger.com/2007/07/03/pattern-hate-singleton/
You do not have to agree with it, but it is better to
add these issues to your check-list, IMHO.
HTH
Jiang
Jason Hise
> Perhaps you are referring to what the FAQ calls C++'s "static -
> initialization- order" fiasco (in which the order that statically-
> allocated objects are initialized - is not always specified in C++).
For what it's worth, there is a decent solution which works in most
cases to resolve initialization order problems. If you can identify
the dependencies that exist between global objects, then you can use a
member variable or inheritance from a helper class to ensure that
dependencies get constructed before the objects that depend upon them,
and therefore also get destroyed after the dependent objects.
See http://www.entropygames.net/text/articles/globals.pdf for more
details.
Jason
Jason Hise
> Perhaps you are referring to what the FAQ calls C++'s "static -
> initialization- order" fiasco (in which the order that statically-
> allocated objects are initialized - is not always specified in C++).
For what it's worth, there is a decent solution which works in most
cases to resolve initialization order problems.
http://www.entropygames.net/text/articles/globals.pdf
Jason
{ Mod note: An HTML version of Jason's article is available at <url:
http://www.entropygames.net/globals.htm>. Summarizing from the last section:
template <typename Type>
struct depends_on
{
depends_on()
{
Type::instance();
}
};
where 'instance' is a convention for accessing a singleton, with example
class connection
: depends_on<logger>
{
private:
~connection()
{
// Close connection here.
logger::instance()
.log("Connection closed.");
}
public:
static connection & instance()
{
static connection inst;
return inst;
}
};
- mod }
Chris Thomasson
news:50d843a1-9252-4c47...@z6g2000pre.googlegroups.com...
> On Jul 24, 3:10 pm, JoshuaMaur...@gmail.com wrote:
>> On Jul 24, 12:58 pm, Greg Herlihy <gre...@mac.com> wrote:
>>
>> > Why call "new" to allocate the singleton in the first place? Wouldn't
>> > the more obvious solution be to avoid "new" and "delete" by having the
>> > singleton be statically - instead of dynamically - allocated? In fact,
>> > the "classic" singleton pattern takes such an approach:
>>
>> I'd suggest seeing the FAQ, and the static deinitialization order
>> fiasco. As others have said in this thread, it's not a simple problem
>> with a simple solution, especially in multithreaded environments.
>
> Anyone who follows your suggestion and reads the C++ FAQ - will learn
> that there is no such thing as the "static deinitialization order
> fiasco" in C++ (at least as far as the FAQ is concerned). After all,
> the destruction of static objects is well-defined (static objects are
> destroyed in the reverse order of their construction.)
Joshua mentioned multi-threaded environment; the singleton code you posted
is NOT thread-safe. You have several issues to deal with, and AFAICT you
addressed absolutely none of them...
Rune Allnor
> > Is there a reference to this 'classic' singleton pattern
> > somewhere? I based my suggestion on the recipe in Gamma & al.
>
> For the above "Meyers-Singleton",
Ah, the name is familiar...
> please read "More Effective C++"
> by Scott Meyers. Sure you've read it, right? :-)
I've *browsed* it, yes. Part of the problem is to find one's
way between all the books. Without the hands-on programming
experience you don't see the significance of what they tell.
Without knowing the books you don't get much done in programming.
A classic CATCH 22.
> Singleton is a special pattern, here "special" means, it is quite easy
> to understand how it works at the first glance, but is extremely
> difficult to make it really work. Yes, as other said, there is no such
> a silver-bullet can solve all the problems.
>
> Please have a look at Loki::Singleton at
>
> http://loki-lib.svn.sourceforge.net/viewvc/loki-lib/trunk/include/lok...
>
> , maybe it is not the singleton as you expected. :-)
I did have a look at the Alexandrescu book. Frankly, I was a bit
disappointed with it - I actually understood what he tried to
achieve if not all the details in the solutions. Of course, that
has to do with some of the reviews I have seen, where it is
stressed that the book is intended for an intelligent audience.
That's a statement one doesn't see too often.
> So my suggestion is, well, check your targets carefully and find
> the necessary trade-offs to make the life easier. For example,
>
> * If you can make sure you will not do mulit-threading
> in your program, then lock issues can be saved.
No multithreading in my programs. Yet.
> * If you can change your definition of "memory leak",
> then you do not have to delete the singleton in the heap,
> since most of the OS will handle this issue for you.
> (But this does not apply to other resource, such as
> GUI handles, .etc)
This was what I expected to happen. The problem was that
Visual Studio spewed lots of error messages when the singleton
and some of its internal variables were not explicitly deleted.
A bit annoying, not to mention that there were a couple of other
instances which were de facto memory leaks hidden deep inside
that cascade of error messages.
> P.S. Please make sure you really need the Singleton,
> someone believes Singleton is an Anti-Pattern.
>
> http://blogs.msdn.com/scottdensmore/archive/2004/05/25/140827.aspx
> http://tech.puredanger.com/2007/07/03/pattern-hate-singleton/
>
> You do not have to agree with it, but it is better to
> add these issues to your check-list, IMHO.
I'll have a look at them.
Rune
Chris Thomasson
"Chris Thomasson" <x...@xxx.xxx> wrote in message
news:g6m5fr$d30$1...@aioe.org...
ARGHGHGH!
> Here is an atomically thread-safe singleton implementation using pthreads,
> x86, MSVC and
> the double-checked locking pattern:
> ___________________________________________________________________________
> template<typename T>
> struct singleton {
> static T* instance() {
> static T* volatile this_ptr = NULL;
> T* ptr = (T*)atomic::ldptr_acq((void* volatile*)&this_ptr);
> if (! ptr) {
> static pthread_mutex_t this_mtx = PTHREAD_MUTEX_INITIALIZER;
> mutex_guard lock(&this_mtx);
> if (! (ptr = this_ptr)) {
> static T this_instance;
> ptr = this_ptr = (T*)atomic::stptr_rel(
> (void* volatile*)&this_ptr, &this_instance
> );
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Ummm... There is a "harmless" condition here... Notice how I atomically
store into `this_ptr' and then make an assignment to it! The line above
should read as:
ptr = (T*)atomic::stptr_rel(
(void* volatile*)&this_ptr, &this_instance
);
> }
> }
> assert(ptr);
> return ptr;
> }
> };
>
[...]
Here is the code in full with that error fixed:
___________________________________________________________________________
#include <iostream>
#include <cassert>
#include <pthread.h>
class mutex_guard {
pthread_mutex_t* const m_mtx;
public:
mutex_guard(pthread_mutex_t* const mtx)
: m_mtx(mtx) {
pthread_mutex_lock(m_mtx);
}
~mutex_guard() throw() {
pthread_mutex_unlock(m_mtx);
}
};
namespace atomic {
__declspec(naked)
static void*
ldptr_acq(void* volatile*) {
_asm {
MOV EAX, [ESP + 4]
MOV EAX, [EAX]
RET
}
}
__declspec(naked)
static void*
stptr_rel(void* volatile*, void* const) {
_asm {
MOV ECX, [ESP + 4]
MOV EAX, [ESP + 8]
MOV [ECX], EAX
RET
}
}
}
template<typename T>
struct singleton {
static T* instance() {
static T* volatile this_ptr = NULL;
T* ptr = (T*)atomic::ldptr_acq((void* volatile*)&this_ptr);
if (! ptr) {
static pthread_mutex_t this_mtx = PTHREAD_MUTEX_INITIALIZER;
mutex_guard lock(&this_mtx);
if (! (ptr = this_ptr)) {
static T this_instance;
ptr = (T*)atomic::stptr_rel(
(void* volatile*)&this_ptr, &this_instance
);
}
}
assert(ptr);
return ptr;
}
};
struct foo {
foo() {
std::cout << "(" << this << ")->foo::foo()" << std::endl;
}
~foo() throw() {
std::cout << "(" << this << ")->foo::~foo()" << std::endl;
}
};
int main() {
foo* ptr1 = singleton<foo>::instance();
foo* ptr2 = singleton<foo>::instance();
foo* ptr3 = singleton<foo>::instance();
assert(ptr1 == ptr2 && ptr2 == ptr3);
return 0;
}
___________________________________________________________________________
Sorry about that!
;^(...
> This should work fine. Any thoughts?
Chris Thomasson
news:g6m5fr$d30$1...@aioe.org...
One more thing about the BROKEN code I posted, the lock needs to be
singular. In other words, there needs to be a single lock for all the
singletons to use. If this is not the case, then locking order violations
will occur!
I have a solution, but I am not ready to post it yet. Give me about 24
hours.
Chris Thomasson
news:g6m5fr$d30$1...@aioe.org...
Here is a thought on the code I posted: The mutex should be recursive.
Chris Thomasson
news:g6m5fr$d30$1...@aioe.org...
Here is an atomically thread-safe singleton using pthreads, x86, MSVC and
the double checked locking pattern:
~mutex_guard() throw() {
pthread_mutex_unlock(m_mtx);
}
};
ptr = this_ptr = (T*)atomic::stptr_rel(
(void* volatile*)&this_ptr, &this_instance
);
}
}
assert(ptr);
return ptr;
}
};
struct foo {
foo() {
std::cout << "(" << this << ")->foo::foo()" << std::endl;
}
~foo() throw() {
std::cout << "(" << this << ")->foo::~foo()" << std::endl;
}
};
int main() {
foo* ptr1 = singleton<foo>::instance();
foo* ptr2 = singleton<foo>::instance();
foo* ptr3 = singleton<foo>::instance();
assert(ptr1 == ptr2 && ptr2 == ptr3);
return 0;
}
___________________________________________________________________________
This works well... Any thoughts?
Joshua...@gmail.com
wrote:
Not quoting his large block of code.
You may be pushing the bounds of comp.lang.c++.moderated. I cannot
read the code, and not because of any lack of indentation or such. I
cannot read it because it contains too many non-standard things, such
as __declspec. The short version is, using standard threading tools
(aka pthreads et al, not c++ standard, at least not 2003), double
checked locking does not work. Once you wrestle it to the point of
working, which you may or may not have done successfully, you might as
well have just put a single lock then check algorithm, as that is what
it will compile down to. Here
http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf
is an excellent paper describing it much better than I could.