Destruction of Singleton class
Jeremy Lamplough
Helm, Johnson, Vlissides), defined as,
class Singleton {
public:
static Singleton* Instance();
private:
Singleton();
static Singleton* _instance
};
which is implemented as
Singleton* Singleton::_instance = 0;
Singleton* Singleton::Instance() {
if (_instance == 0) {
_instance = new Singleton;
}
return _instance;
}
Should the destructor also be private? As this stands, many clients can
have a pointer to the same class. The way I see it, there should be a
static reference count which keeps a count of how many clients have a
pointer to the singleton object and there should be be a de-instance
method which decrements this counter and if it gets to zero, destructs
the object.
Is there a better way of doing it?
I'd appreciate your input,
Thanks,
Jeremy
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
[ about comp.lang.c++.moderated. First time posters: do this! ]
Bill Klein
>Should the destructor also be private? As this stands, many clients can
>have a pointer to the same class. The way I see it, there should be a
>static reference count which keeps a count of how many clients have a
>pointer to the singleton object and there should be be a de-instance
>method which decrements this counter and if it gets to zero, destructs
>the object.
There's no need for this. The constructor gets called exactly
once, when the static instance is allocated. After that you're
just dealing with pointers/references to that same instance.
In the end the destructor is called exactly once (on program
termination, since it's a static instance). As simple as that
(unless I've missed something :).
- Bill Klein <bi...@orbit.org>
Bill Klein
>Singleton* Singleton::Instance() {
> if (_instance == 0) {
> _instance = new Singleton;
> }
> return _instance;
>}
Woops. I responded to quickly before. :) I hadn't noticed
this funky 'new' business. Because of that the destructor
will never get called at all no matter what (unless you call
'delete' somewhere). If _instance was simply a static
Singleton (instead of a static pointer to one), what I
said in my other letter would apply.
G.B.
reference instead of pointer, so the Singleton instance is created and
destroyed by compiler automatically. Caveat: you should not use Instance in
initialization of other static variables, because you can't guarantee the
order of creation.
Here is the code:
class Singleton {
public:
static Singleton* Instance() {return &instance;}; // inlined to save time
private:
Singleton() {/* whatever you need */};
~Singleton(){/* whatever you need */};
operator = (const Singleton&);
Singleton(const Singleton&);
static Singleton& instance;
};
Singleton& Singleton::instance=Singleton();
void foobar()
{
Singleton* s = Singleton::Instance(); // OK
// Singleton* s1 = new Singleton(); // error
// Singleton s2; // error
// Singleton s3(*s1); //error
// delete s; // error
}
Jeremy Lamplough <jlam...@syd.asc.corp.mot.com> wrote in message
news:375DA657...@syd.asc.corp.mot.com...
> If we have a singleton class (as shown in "design patterns", Gamma,
[snip...]
>
> Is there a better way of doing it?
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
Daniel T.
<jlam...@syd.asc.corp.mot.com> wrote:
>If we have a singleton class (as shown in "design patterns", Gamma,
>Helm, Johnson, Vlissides), defined as,
>
>class Singleton {
>public:
> static Singleton* Instance();
>private:
> Singleton();
> static Singleton* _instance
>};
>
>
>which is implemented as
>
>Singleton* Singleton::_instance = 0;
>
>Singleton* Singleton::Instance() {
> if (_instance == 0) {
> _instance = new Singleton;
> }
> return _instance;
>}
>
>
>Should the destructor also be private?
I doesn't have to be... The destructor is never called...
> As this stands, many clients can
>have a pointer to the same class. The way I see it, there should be a
>static reference count which keeps a count of how many clients have a
>pointer to the singleton object and there should be be a de-instance
>method which decrements this counter and if it gets to zero, destructs
>the object.
>
>Is there a better way of doing it?
>
You would only need to do this if you want to get rid of the singleton
before the end of the program.
Probably the best way to do that would be to have a concrete
SingletonGuard class that users instantiate when they want to get to the
Singleton. Something like this:
class Singleton
{
friend SingletonGuard;
public:
// Useful (non-static) methods defined here
private:
static Singleton* Instance(); // Create theirInstance if needed and
// increment count
static Singleton* Deinstance(); // decrement count, destroy
// instance if needed
Singleton();
~Singleton();
static Singleton* theirInstance;
static short count;
};
class SingletonGuard
{
public:
SingletonGuard(); // Call Singleton::Instance and store
// result in itsPrisioner
~SingletonGuard(); // Call Singleton::Deinstance
Singleton* operator->(); // Used to call Singleton's Methods
private:
const SingletonGuard& operator=(const SingletonGuard&); // Don't call
this
Singleton* itsPrisioner;
};
Nicola Musatti
...
> Should the destructor also be private? As this stands, many clients
a static reference count which keeps a count of how many clients
> have a pointer to the singleton object and there should be be a
> de-instance method which decrements this counter and if it gets to
> zero, destructs the object.
Personally I see the Singleton pattern as a sort of replacement for
global variables, so I expect all calls to your Instance method to
return a pointer to the same instance of the class and not just to limit
the number of existing instances.
The way I do it is to delete the instance in a static member function
and register the latter to be called upon program exit. This is not 100%
safe, but it is enough for me.
> class Singleton {
> public:
> static Singleton* Instance();
> private:
> Singleton();
static void deleteSingleton();
> static Singleton* _instance
> };
>
> which is implemented as
>
> Singleton* Singleton::_instance = 0;
>
> Singleton* Singleton::Instance() {
> if (_instance == 0) {
> _instance = new Singleton;
atexit(Singleton::deleteSingleton);
> }
> return _instance;
> }
void Singleton::deleteSingleton() {
delete _instance;
}
Or even better, provide a base class that maintains a static list of the
created Singletons and destroys them in a single static member.
Best regards,
Nicola Musatti
Nigel Eke
>
> If we have a singleton class (as shown in "design patterns", Gamma,
> Helm, Johnson, Vlissides), defined as,
If you've read Design Patterns you're bound to enjoy "Pattern Hatching
(Design Patterns Applied)" by Vlissides. It's got 11 pages dedicated to
the very topic of destroying a singleton - very enlightening.
Nigel
Luis Coelho
<jlam...@syd.asc.corp.mot.com> uttered the following words:
>If we have a singleton class (as shown in "design patterns", Gamma,
>Helm, Johnson, Vlissides), defined as,
[normal singleton, using lazy initiation]
>
>Should the destructor also be private? As this stands, many clients can
>have a pointer to the same class. The way I see it, there should be a
>static reference count which keeps a count of how many clients have a
>pointer to the singleton object and there should be be a de-instance
>method which decrements this counter and if it gets to zero, destructs
>Is there a better way of doing it?
It depends :)
Seriously. The destructor need not be private, since noone should
be calling it anyway. However, making it private makes it a
compile-time mistake for anyone to call it (by deleting the
pointer).
Now, whether to have reference-counted pointers or not depends on
whether you want to delete the object once noone is using it.
Maybe you don't care. If it is a not very large object which does
not hog any important resources you should not care whether it is
deleted whenever noone is using it. So reference-counting is
overhead here (overhead does not imply only that it affects
performance, it also IMHO complicates your code even if just a
little bit).
Another question is whether you want to call delete *at the end
of the program.* Maybe you don't care. If it is something simple
all you do by deleting it is release the memory, but if your
program is exiting you may not care. But sometimes you might like
to do so, sometimes you have to do so. Someone sugested using
atexit functions. That works, but I find this better:
<code>
struct Type
{
// ...
static Type* Instance()
{
if (!ptr) ptr = new Type;
return ptr;
}
private:
static Type* ptr;
struct Type_destroyer {
// private helper type to destoy singleton
~Type_destroyer() { delete Type::ptr; }
// since delete on null-pointer has no effect, this
// works evens if Type::Instance() is never called
};
friend class Type_destroyer; // need be
static Type_destroyer destroyer;
};
</code>
This uses the destructor of a static object (of type
Type_destroyer, which exists solelly for this porpuse) to delete
the object.
Hope this helped,
Luis Coelho.
C++ Programming Language, 3rd Ed. by B. Stroustrup. My exercise answers at:
http://www.geocities.com/SiliconValley/Way/3972/index.html
Helge Penne
>If we have a singleton class (as shown in "design patterns", Gamma,
>Helm, Johnson, Vlissides), defined as,
>
>class Singleton {
>public:
> static Singleton* Instance();
>private:
> Singleton();
> static Singleton* _instance
>};
>
>
>which is implemented as
>
>Singleton* Singleton::_instance = 0;
>
>Singleton* Singleton::Instance() {
> if (_instance == 0) {
> _instance = new Singleton;
> }
> return _instance;
>}
>
>Should the destructor also be private? As this stands, many clients can
>have a pointer to the same class. The way I see it, there should be a
>static reference count which keeps a count of how many clients have a
>pointer to the singleton object and there should be be a de-instance
>method which decrements this counter and if it gets to zero, destructs
>the object.
>
>Is there a better way of doing it?
>
>I'd appreciate your input,
>
>Thanks,
>
>Jeremy
If you need to create the object with "new", then how about:
class Singleton {
public:
static Singleton* Instance();
private:
static std::auto_ptr<Singleton> m_Instance;
};
and:
std::auto_ptr<Singleton> Singleton::m_instance;
Singleton* Singleton::Instance()
{
if (m_Instance.get() == NULL)
m_Instance = new Singleton;
return m_Instance.get();
}
The destructor of auto_ptr<> will ensure that the object is destroyed
when main() returns.
However, i normally prefer the approach in Scott Meyer's "Effective
C++", which is:
class Singleton {
public:
static Singleton& Instance()
{
static Singleton onlyInstance;
return onlyInstance;
}
};
This is much smaller and more elegant, but requires that Singleton has
a default constructor. If you cannot use the default constructor
(because one or more constructor parameters are required), then the
first example might be better since it can be modified to let a
"Create" function create the object. This Create function would the
take one or more parameters that are passed on to the constructor.
The code for Instance would probably be changed to something like:
Singleton* Singleton::Instance()
{
assert( m_Instance.get() != NULL); // Call Create before Instance!
return m_Instance.get();
}
Note that the constructor/destructor would normally be protected or
private (not shown in the example code).
----------------
Helge Penne, MSc
Senior Software Development Engineer, Data Respons AS
To reply, swap the @ and the first dot of my e-mail address.
All opinions expressed are those of the author, not of Data Respons AS
Helge Penne
Penne) wrote:
>Singleton* Singleton::Instance()
>{
> if (m_Instance.get() == NULL)
> m_Instance = new Singleton;
> return m_Instance.get();
>}
Hmm, the constructor for auto_ptr is "explicit", so the code for this
function should have been as follows:
Singleton* Singleton::Instance()
{
if (m_Instance.get() == NULL)
m_Instance = auto_ptr<Singleton>(new Singleton);
return m_Instance.get();
Darin Adler
(Helge Penne) wrote:
> Hmm, the constructor for auto_ptr is "explicit", so the code for this
> function should have been as follows:
>
> Singleton* Singleton::Instance()
> {
> if (m_Instance.get() == NULL)
> m_Instance = auto_ptr<Singleton>(new Singleton);
> return m_Instance.get();
> }
I would do it without creating an auto_ptr temporary:
Singleton* Singleton::Instance()
{
if (m_Instance.get() == NULL)
m_Instance.reset(new Singleton);
return m_Instance.get();
}
-- Darin
Livio Cavallo
class Singleton {
public:
static Singleton* Instance() {
static Sigleton ston_only_object; return &ston_only_object; }
// or this:
// static Singleton& Instance() {
// static Sigleton ston_only_object; return ston_only_object; }
private:
Singleton();
};
Livio Cavallo
Siemel Naran
>class Singleton {
>public:
> static Singleton* Instance() {
> static Sigleton ston_only_object; return &ston_only_object; }
>
> // or this:
> // static Singleton& Instance() {
> // static Sigleton ston_only_object; return ston_only_object; }
>
>private:
> Singleton();
>};
Consider making the destructor private, and the copy constructor and
operator= private and not implemented.
--
----------------------------------
Siemel B. Naran (sbn...@uiuc.edu)
----------------------------------
Andrei Alexandrescu
"Livio Cavallo" <cava...@tin.it> wrote:
> Apart from static objects initialization order issues, what about:
>
> public:
> static Singleton* Instance() {
> static Sigleton ston_only_object; return &ston_only_object; }
>
> // or this:
> // static Singleton& Instance() {
> // static Sigleton ston_only_object; return ston_only_object; }
>
> private:
> Singleton();
> };
Apart from that? Hmmmm... I personally prefer the commented version, for
the following reasons.
We C++ programmers are nervous about pointers. Pointers come together
with the ownership dilemma: who the hack owns this guy? Who's got to
delete it?
For this reason, the Singleton returning a pointer might toggle a
pre-smart pointers reflex in a programmer: they might delete it.
Because references look much more value-like (they sport value syntax,
inlcluding the cute dot operator), programmers aren't that nervous when
they get a reference. Oh, it's a reference, it's callee's trouble to
take care of its lifetime.
Also the code that takes the address of a function return value or takes
the address of a variable and delete's it, look much more suspicious to
a code reviewer. Programmers aren't that sanguine about writing such
code, and in fact some may even not know it's possible to do this.
That's why I think it's better to return a reference in this case.
For the same reason, I advocate disabling the unary
operator&() (address-of) for Singletons. There is only one Singleton,
and there's a single global point of access to it, so there's no need to
take its address at all. Of course, some people will try to lynch me for
that, but I guess I'll outlive it.
Andrei
Sent via Deja.com http://www.deja.com/
Share what you know. Learn what you don't.
Helge Penne
>In article <3765f37...@news.powertech.no>, he...@penne.datarespons.no
>(Helge Penne) wrote:
>
>> Hmm, the constructor for auto_ptr is "explicit", so the code for this
>> function should have been as follows:
>>
>> Singleton* Singleton::Instance()
>> {
>> if (m_Instance.get() == NULL)
>> m_Instance = auto_ptr<Singleton>(new Singleton);
>> return m_Instance.get();
>> }
>
>I would do it without creating an auto_ptr temporary:
>
> Singleton* Singleton::Instance()
> {
> if (m_Instance.get() == NULL)
> m_Instance.reset(new Singleton);
> return m_Instance.get();
> }
>
> -- Darin
I would very much have liked to be able to avoid the auto_ptr
temporary like this, but it does not seem bo be supported by Visual
C++ 6.0. I also checked Stroustrup ("The C++ Prog. L."), third
edition. "release" does not take any parameters, at least not in the
declaration of auto_ptr in section 14.4.2 (page 368 in my book). This
could indicate that the implementation of release that your example
relies on is non-standard.
I must admit that I find it rather strange that auto_ptr does not have
any kind of "set" function, or any defined conversion from the pointer
type, but instead requires this (rather ugly) temporary instead.
- Helge
----------------
Helge Penne, MSc
Senior Software Development Engineer, Data Respons AS
To reply, swap the @ and the first dot of my e-mail address.
All opinions expressed are those of the author, not of Data Respons AS
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
James...@dresdner-bank.com
bi...@orbit.org (Bill Klein) wrote:
> Jeremy Lamplough <jlam...@syd.asc.corp.mot.com> wrote:
> >Should the destructor also be private? As this stands, many clients
can
> >have a pointer to the same class. The way I see it, there should be
a
> >static reference count which keeps a count of how many clients have a
> >pointer to the singleton object and there should be be a de-instance
> >method which decrements this counter and if it gets to zero,
destructs
> >the object.
>
> once, when the static instance is allocated. After that you're
> just dealing with pointers/references to that same instance.
> In the end the destructor is called exactly once (on program
> termination, since it's a static instance). As simple as that
> (unless I've missed something :).
I think his worry is that anyone with a pointer to the object can delete
it. With desasterous consequences for the others which have pointers to
it.
In general, a singleton class should declare all of the following
private: copy constructor, assignment operator, operator new and
operator delete.
As to the problem of when to destruct, using reference counting doesn't
work, because you don't know when your clients loose the reference. And
for that matter, just because you have no more clients doesn't mean that
its time to destruct.
In the classical implementation, the object is simply never destructed.
This is probably the safest solution. My implementations use a static
object, and so are destructed during termination. But I generally code
so that destructors don't use the singleton, so this is not a problem.
I am at present re-evaluating my implementation, as thread safety has
become an issue, and none of the published implementations are truely
thread safe. (The classic double lock, for example, prevents multiple
instances, but allows other threads to see the object in an incomplete
state.)
--
James Kanze mailto:
James...@dresdner-bank.com
Conseils en informatique orientée objet/
Beratung in objekt orientierter
Datenverarbeitung
Ziegelhüttenweg 17a, 60598 Frankfurt, Germany Tel. +49 (069) 63 19 86
27
Sent via Deja.com http://www.deja.com/
Share what you know. Learn what you don't.
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
Helge Penne
wrote:
>he...@penne.datarespons.no (Helge Penne) wrote:
>
>>On 16 Jun 1999 06:22:45 -0400, "Darin Adler" <da...@spies.com> wrote:
>>
>>>I would do it without creating an auto_ptr temporary:
>>>
>>> Singleton* Singleton::Instance()
>>> {
>>> if (m_Instance.get() == NULL)
>>> m_Instance.reset(new Singleton);
> ^^^^^
>>> return m_Instance.get();
>>> }
>>
>>I would very much have liked to be able to avoid the auto_ptr
>>temporary like this, but it does not seem bo be supported by Visual
>>C++ 6.0. I also checked Stroustrup ("The C++ Prog. L."), third
>>edition. "release" does not take any parameters, at least not in the
>>declaration of auto_ptr in section 14.4.2 (page 368 in my book). This
>>could indicate that the implementation of release that your example
>>relies on is non-standard.
>
>ISO standard 20.4.3 lists std::auto_ptr::reset(void *).
Ah! Good. Too bad that it was omitted in Stroustrups book.
>It also mentions std::auto_ptr::release(), but that's a different
>function.
>
>>I must admit that I find it rather strange that auto_ptr does not have
>>any kind of "set" function, or any defined conversion from the pointer
>>type, but instead requires this (rather ugly) temporary instead.
>
>It would be strange, but as that's what reset() is for, it isn't.
>
>However, just checking VC++6, I notice that it isn't mentioned. Either
>PJ Plauger and his team at Dinkumware missed it out, or the spec came
>along a little too late, since I can't see it in the source either.
>
>Alan Bellingham
I'll forward this message it to Dinkumware in case they are not aware
of this omission.
- Helge
----------------
Helge Penne, MSc
Senior Software Development Engineer, Data Respons AS
To reply, swap the @ and the first dot of my e-mail address.
All opinions expressed are those of the author, not of Data Respons AS
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
Andrei Alexandrescu
> In general, a singleton class should declare all of the following
> private: copy constructor, assignment operator, operator new and
> operator delete.
If the constructors and destructors are private, there's no much need
for disabling new and delete. But anyway.
Also I'll recur my obsession that one may want to disable to address-of
operator, too.
> This is probably the safest solution. My implementations use a static
> object, and so are destructed during termination. But I generally
code
> so that destructors don't use the singleton, so this is not a problem.
On the other hand, you cannot be sure a large team will do the same.
Think of a Log object. In destructors people might want to log messages,
and they might create problems, although the need is innocent. This
happened to me so often, I started looking for a way to solve it.
> I am at present re-evaluating my implementation, as thread safety has
> become an issue, and none of the published implementations are truely
> thread safe. (The classic double lock, for example, prevents multiple
> instances, but allows other threads to see the object in an incomplete
> state.)
I know only of "double-checking pattern". That doesn't have the problem
you mention.
Singleton& Singleton::Instance()
{
if (!pInstance_)
SYNCHRONIZED
{
if (!pInstance_)
{
pInstance_ = new T;
// or whatever technique you use
// maybe:
// static Singleton it;
// pInstance_ = ⁢
}
}
}
(SYNCHRONIZED is a macro that emulates the synchronized Java keyword.
It's a pseudo-statement that can be used with or without brackets.)
I don't see how another thread can break into the synchronized portion
of code and see the Singleton in a partial constructed state.
Another potential problem is the dead reference, which occurs when an
object accesses the Singleton after it was destroyed.
This may happen usually, as you mentioned, when the destructor of a
global object (maybe another Singleton) accesses the Singleton.
Unfortunately, the problem appears more often and in more usual
situations than one might think. But fortunately, the dead reference is
easy and unexpensive to detect. Once you detect it, there are a lot of
interesting things you can do. A whole lot.
Andrei
Sent via Deja.com http://www.deja.com/
Share what you know. Learn what you don't.
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
Mike Davis
class Singleton {
public:
static Singleton *Instance()
{
static Singleton theInstance;
return &theInstance;
}
};
Helge Penne wrote:
> On 10 Jun 1999 17:46:01 -0400, Jeremy Lamplough
> <jlam...@syd.asc.corp.mot.com> wrote:
>
> >If we have a singleton class (as shown in "design patterns", Gamma,
> >Helm, Johnson, Vlissides), defined as,
> >
> >class Singleton {
> >public:
> > static Singleton* Instance();
> >private:
> > Singleton();
> > static Singleton* _instance
> >};
> >
> >
> >which is implemented as
> >
> >Singleton* Singleton::_instance = 0;
> >
> >Singleton* Singleton::Instance() {
> > if (_instance == 0) {
> > _instance = new Singleton;
> > }
> > return _instance;
> >}
> >
> >Should the destructor also be private? As this stands, many clients can
> >have a pointer to the same class. The way I see it, there should be a
> >static reference count which keeps a count of how many clients have a
> >pointer to the singleton object and there should be be a de-instance
> >method which decrements this counter and if it gets to zero, destructs
> >the object.
> >
> >Is there a better way of doing it?
> >
> >I'd appreciate your input,
> >
> >Thanks,
> >
> >Jeremy
>
> If you need to create the object with "new", then how about:
>
> class Singleton {
> public:
> static Singleton* Instance();
> private:
> static std::auto_ptr<Singleton> m_Instance;
> };
>
> and:
>
> std::auto_ptr<Singleton> Singleton::m_instance;
>
> Singleton* Singleton::Instance()
> {
> if (m_Instance.get() == NULL)
> m_Instance = new Singleton;
> return m_Instance.get();
> }
>
> The destructor of auto_ptr<> will ensure that the object is destroyed
> when main() returns.
>
> However, i normally prefer the approach in Scott Meyer's "Effective
> C++", which is:
>
> class Singleton {
> public:
> static Singleton& Instance()
> {
> static Singleton onlyInstance;
> return onlyInstance;
> }
> };
>
> This is much smaller and more elegant, but requires that Singleton has
> a default constructor. If you cannot use the default constructor
> (because one or more constructor parameters are required), then the
> first example might be better since it can be modified to let a
> "Create" function create the object. This Create function would the
> take one or more parameters that are passed on to the constructor.
> The code for Instance would probably be changed to something like:
>
> Singleton* Singleton::Instance()
> {
> assert( m_Instance.get() != NULL); // Call Create before Instance!
> return m_Instance.get();
> }
>
> Note that the constructor/destructor would normally be protected or
> private (not shown in the example code).
>
> ----------------
> Helge Penne, MSc
> Senior Software Development Engineer, Data Respons AS
> To reply, swap the @ and the first dot of my e-mail address.
> All opinions expressed are those of the author, not of Data Respons AS
>
Helge Penne
wrote:
>I seem to be missing something here. I'd want to write it as:
>
>class Singleton {
> public:
> static Singleton *Instance()
> {
> static Singleton theInstance;
> return &theInstance;
> }
>};
>
Sure, no problem. It's the same as Scott Meyer's Singleton, except
that you return a pointer instead of a reference. References has many
advantages over pointers, and should (in my opinion) be preferred
whereever possible.
Btw., note that the code is most probably not thread safe before
"Instance" has been called the first time and the object has been
created. If "Instance" is to be used by more than a single thread,
you should either protect the code with a mutex, or call Instance once
in the application's startupcode before the other threads are created,
in order to create the Singleton object.
James...@dresdner-bank.com
he...@penne.datarespons.no (Helge Penne) wrote:
> On 2 Jul 1999 12:17:51 -0400, Mike Davis <mda...@publitec.vnu.com>
> wrote:
>
> >I seem to be missing something here. I'd want to write it as:
> >
> >class Singleton {
> > public:
> > static Singleton *Instance()
> > {
> > static Singleton theInstance;
> > return &theInstance;
> > }
> >};
> >
>
> Sure, no problem. It's the same as Scott Meyer's Singleton, except
> that you return a pointer instead of a reference. References has many
> advantages over pointers, and should (in my opinion) be preferred
> whereever possible.
>
> Btw., note that the code is most probably not thread safe before
> "Instance" has been called the first time and the object has been
> created.
What makes you state this? If the compiler supports threads, the code
should be thread safe. And if the compiler doesn't support threads,
there is no way to make it thread safe.
--
James Kanze mailto:
James...@dresdner-bank.com
Conseils en informatique orientée objet/
Beratung in objekt orientierter
Datenverarbeitung
Ziegelhüttenweg 17a, 60598 Frankfurt, Germany Tel. +49 (069) 63 19 86
27
Sent via Deja.com http://www.deja.com/
Share what you know. Learn what you don't.
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
Nick Ambrose
James...@dresdner-bank.com wrote:
> In article <3781aacd...@news.powertech.no>,
> he...@penne.datarespons.no (Helge Penne) wrote:
> > On 2 Jul 1999 12:17:51 -0400, Mike Davis <mda...@publitec.vnu.com>
> > wrote:
> >
> > >I seem to be missing something here. I'd want to write it as:
> > >
> > >class Singleton {
> > > public:
> > > static Singleton *Instance()
> > > {
> > > static Singleton theInstance;
> > > return &theInstance;
> > > }
> > >};
> > >
> >
> > Sure, no problem. It's the same as Scott Meyer's Singleton, except
> > that you return a pointer instead of a reference. References has many
> > advantages over pointers, and should (in my opinion) be preferred
> > whereever possible.
> >
> > Btw., note that the code is most probably not thread safe before
> > "Instance" has been called the first time and the object has been
> > created.
>
> What makes you state this? If the compiler supports threads, the code
> should be thread safe. And if the compiler doesn't support threads,
> there is no way to make it thread safe.
>
Well, since construction of the static can occur when Instance is called the
first time, isn't it possible that even on a thread-safe compiler, two
threads could enter the function and call the constructor ?
I have always wondered about it and generally assumed that it could in fact
occur that way and have never seen an explicit statement in my compilers
documentation.
Nick
Helge Penne
>In article <3781aacd...@news.powertech.no>,
> he...@penne.datarespons.no (Helge Penne) wrote:
>> On 2 Jul 1999 12:17:51 -0400, Mike Davis <mda...@publitec.vnu.com>
>> wrote:
>>
>> >I seem to be missing something here. I'd want to write it as:
>> >
>> >class Singleton {
>> > public:
>> > static Singleton *Instance()
>> > {
>> > static Singleton theInstance;
>> > return &theInstance;
>> > }
>> >};
>> >
>>
>> Sure, no problem. It's the same as Scott Meyer's Singleton, except
>> that you return a pointer instead of a reference. References has many
>> advantages over pointers, and should (in my opinion) be preferred
>> whereever possible.
>>
>> Btw., note that the code is most probably not thread safe before
>> "Instance" has been called the first time and the object has been
>> created.
>
>What makes you state this? If the compiler supports threads, the code
>should be thread safe. And if the compiler doesn't support threads,
>there is no way to make it thread safe.
The reason I said "probably" was that I have never looked into what
kind of code the static variable declaration generates. If somebody
knows, I'd be very interested.
If I'm not mistaken the standard implies that "theInstance" will be
constructed during the first call to the "Instance" member function.
The code would therefore need to use some kind of flag that signals if
the object has been constructed or not. This flag is probably updated
after construction of the object.
The following scenario could be a problem:
1. Thread A calls "Instance", and the constructor is called to
initialize "theInstance".
2. A pre-emptive contect switch is performed before the flag
indicating a valid "theInstance" object is updated, and thread B
starts to run.
3. Thread B calls "Instance". "theInstance" is constructed, and the
flag updated.
4. A second context switch occurs, and thread A continues. The
construction of "theInstance" by thread A is completed.
In this scenario, "theInstance" is constructed twice, despite the fact
that the standard says that it should only be constructed once. I
would not call this completely thread safe.
To what degree this would be a problem in real life would depend on
the actual implementation of the constructor. It could be completely
harmless, it could cause a memory leak, or it could cause a program
crash immediately or much later. It all depends on the constructor
code.
Unless I'm missing something important (still haven't had my morning
coffee yet), the code could be made thread safe by protecting the code
in "Instance" with a mutex. If you don't want to do this for some
reason, you could simply call "Instance" in the application's startup
code before multiple threads are started. The method should be thread
safe after this first call.
----------------
Helge Penne, MSc
Senior Software Development Engineer, Data Respons AS
To reply, swap the @ and the first dot of my e-mail address.
All opinions expressed are those of the author, not of Data Respons AS
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
James...@dresdner-bank.com
> Well, since construction of the static can occur when Instance is
called the
> first time, isn't it possible that even on a thread-safe compiler, two
> threads could enter the function and call the constructor ?
> I have always wondered about it and generally assumed that it could in
fact
> occur that way and have never seen an explicit statement in my
compilers
> documentation.
The standard says nothing about this, since it doesn't address threading
or thread-safety at all. So basically, you're at the mercy of you
compiler vendor. *I* would consider it a bad implementation, or an
error, if the compiler didn't generate the necessary code for this to
work correctly. But in the absence of a specific statement from the
vendor to this effect, some mistrust is probably not a bad attitude.
--
James Kanze mailto:
James...@dresdner-bank.com
Conseils en informatique orientée objet/
Beratung in objekt orientierter
Datenverarbeitung
Ziegelhüttenweg 17a, 60598 Frankfurt, Germany Tel. +49 (069) 63 19 86
27
Sent via Deja.com http://www.deja.com/
Share what you know. Learn what you don't.
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
Andrei Alexandrescu
James...@dresdner-bank.com wrote:
[about Meyers Singleton]
> What makes you state this? If the compiler supports threads, the code
> should be thread safe. And if the compiler doesn't support threads,
> there is no way to make it thread safe.
The code should not be thread safe even if the compiler supports
threads. And there are a number of ways of making it thread safe using
classic MT techniques.
One that many people like with Singleton is the Double-Checking pattern.
Andrei
James...@dresdner-bank.com
he...@penne.datarespons.no (Helge Penne) wrote:
> On 7 Jul 1999 10:51:20 -0400, James...@dresdner-bank.com wrote:
>
> >In article <3781aacd...@news.powertech.no>,
> > he...@penne.datarespons.no (Helge Penne) wrote:
> >> On 2 Jul 1999 12:17:51 -0400, Mike Davis <mda...@publitec.vnu.com>
> >> wrote:
> >>
> >> >I seem to be missing something here. I'd want to write it as:
> >> >
> >> >class Singleton {
> >> > public:
> >> > static Singleton *Instance()
> >> > {
> >> > static Singleton theInstance;
> >> > return &theInstance;
> >> > }
> >> >};
> >> >
> >>
> >> Sure, no problem. It's the same as Scott Meyer's Singleton, except
> >> that you return a pointer instead of a reference. References has
many
> >> advantages over pointers, and should (in my opinion) be preferred
> >> whereever possible.
> >>
> >> Btw., note that the code is most probably not thread safe before
> >> "Instance" has been called the first time and the object has been
> >> created.
> >
code
> >should be thread safe. And if the compiler doesn't support threads,
> >there is no way to make it thread safe.
>
> kind of code the static variable declaration generates. If somebody
> knows, I'd be very interested.
>
> If I'm not mistaken the standard implies that "theInstance" will be
> constructed during the first call to the "Instance" member function.
> The code would therefore need to use some kind of flag that signals if
> the object has been constructed or not. This flag is probably updated
> after construction of the object.
This is a typical implementation. What the standard requires is that
the constructor be called if the object has not been constructed, and
that the object is only considered constructed after the constructor has
terminated.
> The following scenario could be a problem:
>
> 1. Thread A calls "Instance", and the constructor is called to
> initialize "theInstance".
> 2. A pre-emptive contect switch is performed before the flag
> indicating a valid "theInstance" object is updated, and thread B
> starts to run.
> 3. Thread B calls "Instance". "theInstance" is constructed, and the
> flag updated.
> 4. A second context switch occurs, and thread A continues. The
> construction of "theInstance" by thread A is completed.
>
> In this scenario, "theInstance" is constructed twice, despite the fact
> that the standard says that it should only be constructed once. I
> would not call this completely thread safe.
Since the presence of the flag is internal to the compiler, and takes
place behind the users back, I would expect the compiler to take the
necessary precautions. Typically, this would mean that the compiler
should arrange for the necessary mutex's, etc.
--
James Kanze mailto:
James...@dresdner-bank.com
Conseils en informatique orientée objet/
Beratung in objekt orientierter
Datenverarbeitung
Ziegelhüttenweg 17a, 60598 Frankfurt, Germany Tel. +49 (069) 63 19 86
27
James...@dresdner-bank.com
Andrei Alexandrescu <andre...@hotmail.com> wrote:
> In article <7lv6jf$85f$1...@nnrp1.deja.com>,
> James...@dresdner-bank.com wrote:
> [about Meyers Singleton]
code
> > should be thread safe. And if the compiler doesn't support threads,
> > there is no way to make it thread safe.
>
> threads. And there are a number of ways of making it thread safe using
> classic MT techniques.
This is debatable. Since the actual code to call the constructor, etc.,
is *not* part of user code, I would expect the compiler to take the
necessary steps. On the other hand, it does mean that you pay for the
thread safety even if you don't need it. (You might, for example, know
that you obtain the first instance before having started threading, and
so know that thread-safety is irrelevant.)
> One that many people like with Singleton is the Double-Checking
pattern.
I presume you are referring to something like:
Singleton&
Singleton::instance()
{
if ( myInstance == null ) {
lock() ;
if ( myInstance == null ) {
myInstance = new Singleton() ;
}
unlock() ;
}
return *myInstance ;
}
Regretfully, this is not thread safe. There is no risk of creating two
instances, but there is a risk of accessing the single instance before
it has been fully constructed.
Doug Harrison
>I presume you are referring to something like:
>
> Singleton&
> Singleton::instance()
> {
> if ( myInstance == null ) {
> lock() ;
> if ( myInstance == null ) {
> myInstance = new Singleton() ;
> }
> unlock() ;
> }
> return *myInstance ;
> }
>
>Regretfully, this is not thread safe. There is no risk of creating two
>instances, but there is a risk of accessing the single instance before
>it has been fully constructed.
As I understand it, it's always thread-safe on uniprocessor machines, and
it's only unsafe on multiprocessor machines that have aggressive memory
architectures and require memory barriers. Here's what I believe is a safe
version for the latter type of system:
X& general_instance()
{
static X* volatile px;
if (!px)
{
Lock lock(critsec);
// 1. Implicit MB here due to entry into critsec; important
// if this thread doesn't create the X.
if (!px)
{
X* temp = new X;
MB; // 2
px = temp;
}
// 3. Implicit MB here due to release of critsec. I think this
// plus the MB at (1) is what ensures that the write to px is
// always visible in the test just above when a second thread
// blocked on the critsec is released.
}
else
MB; // 4
return *px;
}
From what I've been told, you can't get rid of the reader's MB at (4), and
if the thread turns out to be a reader, you need an MB at (1). The MB at (2)
is ensures that if px is visibly non-zero in another thread, *px is as well,
provided that the other thread also does an MB (1, 4).
If you have thread-local storage, I expect you can avoid the overhead of
general_instance for all but the first call:
X& instance()
{
static X* thread-local px;
if (!px)
px = &general_instance();
return *px;
}
In this case, you might as well simplify general_instance() to always enter
the critical section:
X& general_instance()
{
static X* volatile px;
Lock lock(critsec);
if (!px)
px = new X;
return *px;
}
As long as you can arrange for the critical section object to be initialized
before any thread needs it, I don't see any problem with the above. (For
example, I can think of a couple of ways to do this in Visual C++, one using
DLLs and the other a #pragma, such that other static duration data
initializations could call general_instance().)
--
Doug Harrison
dHar...@worldnet.att.net
James...@dresdner-bank.com
> >I presume you are referring to something like:
> > Singleton&
> > Singleton::instance()
> > {
> > if ( myInstance == null ) {
> > lock() ;
> > if ( myInstance == null ) {
> > myInstance = new Singleton() ;
> > }
> > unlock() ;
> > }
> > return *myInstance ;
> > }
> >Regretfully, this is not thread safe. There is no risk of creating
two
> >instances, but there is a risk of accessing the single instance
before
> >it has been fully constructed.
> As I understand it, it's always thread-safe on uniprocessor machines,
Not at all.
The standard ignores threading completely; the code is standard conform,
and perfectly safe in a single threaded environment. Beyond that, we
have to speculate a little. However, the standard is quite clear that
the compiler can generate pretty much anything it wants, as long as the
"observable behavior" is respected. The order of writes to non-volatile
variables, however is *not* observable behavior, and in fact, it is
frequent for optimizers -- even primitive optimizers -- to rearrange
them. (The Lattice C compiler, circa early 80's, for example, would
often generate the writes in an arbitrary order.)
For most compilers, if the constructor is not inline, you will probably
not encounter problems. If the constructor is inline, however, or the
compiler is particularly aggressive, and optimizes across function
boundaries, it is entirely possible, and in certain cases probable, that
the writes in the constructor which initialize the Singleton object
occur after the write to myInstance. In this case, the following
scenario is possible: myInstance written, change threads, new thread
tests myInstance for null, finds it non-null, doesn't try the lock, and
goes on to use the non-initialized object.
I'm not sure how modern compilers handle this, but forbidding the
reordering of writes in general would represent a significant loss of
optimization possibilities.
I'm not that familiar with the semantics of memory barriers; I would
expect that much of this would depend on the actual machine. Still, if
we assume the "standard" definition, that all programmed accesses before
the barrier occur before any of the programmed accesses after the
barrier, I think that the only problem for the reading thread is that it
might use an old (thus null) value of px, and enter the critical section
even when this would not otherwise be necessary. At least for a single
processor.
>From a practical point of view, the simplest solution is probably to
synchronize the constructor, so that it runs in its own critical
section. This is generally not the most efficient solution, but since
the constructor is only called once anyway, it should be acceptable.
> If you have thread-local storage, I expect you can avoid the overhead
of
> general_instance for all but the first call:
>
> X& instance()
> {
> static X* thread-local px;
> if (!px)
> px = &general_instance();
> return *px;
> }
>
> In this case, you might as well simplify general_instance() to always
enter
> the critical section:
If not, I don't think you've gained anything.
This is certainly the simplest solution, but if you are frequently
starting new threads which use the singleton, it may not be particularly
efficient.
> X& general_instance()
> {
> static X* volatile px;
> Lock lock(critsec);
> if (!px)
> px = new X;
> return *px;
> }
>
> As long as you can arrange for the critical section object to be
initialized
> before any thread needs it, I don't see any problem with the above.
(For
> example, I can think of a couple of ways to do this in Visual C++, one
using
> DLLs and the other a #pragma, such that other static duration data
> initializations could call general_instance().)
Depending on the code, the easiest *and* the most efficient solution
might be to just ensure that the singleton is initialized before
starting any threads.
--
James Kanze mailto:
James...@dresdner-bank.com
Conseils en informatique orientée objet/
Beratung in objekt orientierter
Datenverarbeitung
Ziegelhüttenweg 17a, 60598 Frankfurt, Germany Tel. +49 (069) 63 19 86
27
Sent via Deja.com http://www.deja.com/
Share what you know. Learn what you don't.
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
Doug Harrison
>In article <379c9387...@netnews.worldnet.att.net>,
> dHar...@worldnet.att.net (Doug Harrison) wrote:
>> As I understand it, it's always thread-safe on uniprocessor machines,
>
>Not at all.
>
>I'm not sure how modern compilers handle this, but forbidding the
>reordering of writes in general would represent a significant loss of
>optimization possibilities.
That's a good point. I've seen the MP issues discussed now and then, but not
the possibility of the compiler reordering writes. It looks like the
Double-Checked Locking Pattern (DCLP) as usually stated has a fundamental
problem. Besides making the ctor non-inline, I expect you could fix it by
saying:
px = CreateX(); // CreateX returns new X
As long as the compiler doesn't inline CreateX(), this should be fine, and
you won't have to worry about the inline status of the ctor.
>I'm not that familiar with the semantics of memory barriers; I would
>expect that much of this would depend on the actual machine. Still, if
>we assume the "standard" definition, that all programmed accesses before
>the barrier occur before any of the programmed accesses after the
>barrier, I think that the only problem for the reading thread is that it
>might use an old (thus null) value of px, and enter the critical section
>even when this would not otherwise be necessary. At least for a single
>processor.
I thought that for a uniprocessor system, once px was written, it's visible
to all threads, and the memory barriers are unnecessary and have no useful
effect. But on an aggressive MP system, writes to px by one CPU aren't
necessarily observable on other CPUs, at least not right away. So, it seems
like the MP system is the one susceptible to finding a stale value of px.
>From a practical point of view, the simplest solution is probably to
>synchronize the constructor, so that it runs in its own critical
>section. This is generally not the most efficient solution, but since
>the constructor is only called once anyway, it should be acceptable.
I don't see how that would work. If the object has static duration, it seems
like it could be constructed and registered for destruction multiple times,
unless the compiler supports thread-safe initialization of such objects,
which would make all this unnecessary. And if you use the DCLP and new,
what's to prevent the compiler from assigning to the pointer after calling
operator new but before calling the ctor?
>> If you have thread-local storage, I expect you can avoid the overhead
>> of general_instance for all but the first call:
>> In this case, you might as well simplify general_instance() to always
>>enter the critical section:
>
>If not, I don't think you've gained anything.
I think you have. Except for the very first call, you no longer have to
perform even an MB.
>Depending on the code, the easiest *and* the most efficient solution
>might be to just ensure that the singleton is initialized before
>starting any threads.
True, but sometimes you want to delay creation until it's needed.
--
Doug Harrison
dHar...@worldnet.att.net
James...@dresdner-bank.com
> >In article <379c9387...@netnews.worldnet.att.net>,
> > dHar...@worldnet.att.net (Doug Harrison) wrote:
> >> As I understand it, it's always thread-safe on uniprocessor
machines,
> >Not at all.
> >I'm not sure how modern compilers handle this, but forbidding the
> >reordering of writes in general would represent a significant loss of
> >optimization possibilities.
> That's a good point. I've seen the MP issues discussed now and then,
> but not the possibility of the compiler reordering writes. It looks
> like the Double-Checked Locking Pattern (DCLP) as usually stated has a
> fundamental problem. Besides making the ctor non-inline, I expect you
> could fix it by saying:
> px = CreateX(); // CreateX returns new X
> As long as the compiler doesn't inline CreateX(), this should be fine,
> and you won't have to worry about the inline status of the ctor.
And how do you prevent the inlining? I've seen at least one compiler
which will inline across module boundaries, without anything being
declared inline.
> >I'm not that familiar with the semantics of memory barriers; I would
> >expect that much of this would depend on the actual machine. Still,
> >if we assume the "standard" definition, that all programmed accesses
> >before the barrier occur before any of the programmed accesses after
> >the barrier, I think that the only problem for the reading thread is
> >that it might use an old (thus null) value of px, and enter the
> >critical section even when this would not otherwise be necessary. At
> >least for a single processor.
> I thought that for a uniprocessor system, once px was written, it's
> visible to all threads, and the memory barriers are unnecessary and
> have no useful effect. But on an aggressive MP system, writes to px by
> one CPU aren't necessarily observable on other CPUs, at least not
> right away. So, it seems like the MP system is the one susceptible to
> finding a stale value of px.
Yes. I'm not too familiar with the terminology. Maybe what I'm
thinking of has a different name. It is *not* a hardware dispositive,
but rather something that the compiler knows. It represents a point in
the program; The compiler will not allow a write or a read to cross the
barrier when reorganizing code.
For simple compilers, generating a real function call is effectively
such a barrier. I have actually used a compiler, however, which
recognized system functions as such, and did not force the writes of
variables whose address was not being passed to the system function. In
a multi-threaded environment, such a compiler would have to handle the
calls to the locking functions differently.
The sum of the matter is, of course, that your compiler must document
what it considers as such a barrier, and you must program in such a way
that something the compiler will consider as a barrier appears between
the return from the constructor and the assignment of the address to the
pointer.
> >From a practical point of view, the simplest solution is probably to
> >synchronize the constructor, so that it runs in its own critical
> >section. This is generally not the most efficient solution, but
> >since the constructor is only called once anyway, it should be
> >acceptable.
> I don't see how that would work. If the object has static duration, it
> seems like it could be constructed and registered for destruction
> multiple times, unless the compiler supports thread-safe
> initialization of such objects, which would make all this
> unnecessary. And if you use the DCLP and new, what's to prevent the
> compiler from assigning to the pointer after calling operator new but
> before calling the ctor?
I elided too much. You still use the double lock idiom, but with a
synchronized constructor.