Re: Singletons and destructors
"Chris Thomasson" <xxx@xxx.xxx> wrote in message
news:g6m5fr$d30$1@aioe.org...
"Greg Herlihy" <greghe@mac.com> wrote in message
news:50d843a1-9252-4c47-b35e-6ae2234c117b@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...
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?
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]