class T
{
public:
static T* instance();
private:
T() {}
~T() {}
static T* smInstance;
static VMutex smMutex;
};
// T.cpp:
T* T::smInstance = NULL;
VMutex T::smMutex;
T* T::instance()
{
VMutexLocker lock(&smMutex);
if (smInstance == NULL)
smInstance = new T();
return smInstance;
}
Thread Safety
Author
The code below is from an article on the singleton pattern and it said, "If you are sure that the first call to instance() will be made in a single-threaded mode, such as during static initialization, then you don't need the lock."
But without the lock, many threads could potentially be reading smInstance simultaneously.
Would this only be a thread safety issue for an SMP machine?
Thanks.
smInstance can only be written to if smInstance is null. If smInstance is initialized to a non-null value before any additional threads are created, then smInstance can be safely read by multiple threads since no thread will ever try writing to it. (The stuff pointed to by smInstance is another story, however.)
Those provisions simply ensure that you don't leak memory through getInstance().
The rest of your implementation will still need to be synchronized appropriately, but that depends on your implementation.
The rest of your implementation will still need to be synchronized appropriately, but that depends on your implementation.
This is a huge reason why objects that maintain a state shouldn't be singletons - you need to apply synchronization to *every single access* to that singleton, even if you know you're already locked.
Yes, you need to lock access to singleton instance as well. This issue became painfully apparent in Java, where singletons were used for too many types. Not only that, but issues with synchronization (called double-checked locking) were exposed as critical flaws, leading to resource duplication.
Non-const static members with state have no place in multi-threaded applications. There's enough problems with synchronization of transient state as it is.
The only place where singletons can still be used is in purely single-threaded context, and, in case of threads, by providing each thread its own copy of the resource (impossible with pure singletons).
Then you don't need a singleton, but a regular static variable.
Singletons are define by being created on first use - that is all that singleton is. Take that away, and you no longer need them.
Yes, you need to lock access to singleton instance as well. This issue became painfully apparent in Java, where singletons were used for too many types. Not only that, but issues with synchronization (called double-checked locking) were exposed as critical flaws, leading to resource duplication.
Non-const static members with state have no place in multi-threaded applications. There's enough problems with synchronization of transient state as it is.
The only place where singletons can still be used is in purely single-threaded context, and, in case of threads, by providing each thread its own copy of the resource (impossible with pure singletons).
Quote:such as during static initialization, then you don't need the lock."
Then you don't need a singleton, but a regular static variable.
Singletons are define by being created on first use - that is all that singleton is. Take that away, and you no longer need them.
Author
Quote:
smInstance can only be written to if smInstance is null. If smInstance is initialized to a non-null value before any additional threads are created, then smInstance can be safely read by multiple threads since no thread will ever try writing to it.
I understand that no other threads will try writing to smInstance because it will be set in a single-threaded mode before any other threads are created. However, in the below line in the instance function, smInstance may be being read from memory by many threads at the same time who want to grab the pointer to the singleton.
if (smInstance == NULL)Aren't you suppose to synchronize all access to any shared data?
Quote:
(The stuff pointed to by smInstance is another story, however.)
So, I was wondering if the below code would be thread safe. These threads would be created after smInstance is set.
// Thread 1T* pT = T::instance();pT->func();// Thread 2T* pT = T::instance();pT->func();Suppose func doesn't even modify the state of the singleton. It just reads a member variable and returns its value. The above code would still not be thread safe because many threads could be reading the member variable at the same time.
Is this true?
Quote:So, I was wondering if the below code would be thread safe. These threads would be created after smInstance is set.
Who said smInstance was set?
If you, then you don't need a singleton. You know the variable has been initialized, so you no longer need a singleton.
In a singleton pattern, you do not know when (if at all) the instance will be set. If you can control the life-cycle, then it's not a singleton anymore.
The above is abuse of abuse of singletons. First, you statically initialize them. Second, you override thread-safety based on faulty assumptions - singletons that refer to other singletons will no longer have "on first use" guarantee, and that happens instantly in Singleton-based design.
In that case, your singleton is better just:
static T t( ... );You'll only call T members after t has been initialized. No need for getInstance() method.
Talking about initializing the singleton to contain a value is like talking about command pattern being a factory. They are different things - singleton makes no guarantee about existence of its content.
Quote:The code below is from an article on the singleton pattern and it said, "If you are sure that the first call to instance() will be made in a single-threaded mode, such as during static initialization, then you don't need the lock."
This another flaw - static initialization isn't guaranteed to succeed - you do not have any guarantee by language or anyone else (static initialization order fiasco). Singletons are popular since they eliminate this problem. Removing "on first use" brings you back to original problem.
If you want to avoid locks, then CAS approach should work:
X *getInstance(){ if (smInstance == NULL) { 1) X *x = new X(); // if smInstance is null, set it to x if ( !CAS( smInstance, null, x ) ) { 2) // smInstance wasn't null, // someone modified in between 1) and 2), // so delete local copy delete x; } } return smInstance;}volatile X *smInstance;Unfortunately, CAS isn't necessarily supported on all hardware.
If you want to make Singleton-based design, you need to make them thread-safe.
If you want to rely on static initialization, don't use singletons at all.
Author
Quote:Who said smInstance was set?
If you, then you don't need a singleton. You know the variable has been initialized, so you no longer need a singleton.
In a singleton pattern, you do not know when (if at all) the instance will be set. If you can control the life-cycle, then it's not a singleton anymore.
Suppose T is expensive to create or controls some device, then you would need T to be a singleton. You know for sure that your application is going to use T. So, at some point in time it needs to be created.
Why is T no longer a singleton if you can control the life-cycle?
You still need to guarantee that only one instance of T can be created.
Quote:
In that case, your singleton is better just:
static T t( ... );
You'll only call T members after t has been initialized. No need for getInstance() method.
You could do this but it is no longer a singleton. How can you enforce that only one instance of T is created?
Quote:This another flaw - static initialization isn't guaranteed to succeed - you do not have any guarantee by language or anyone else (static initialization order fiasco). Singletons are popular since they eliminate this problem. Removing "on first use" brings you back to original problem.
I read about that static initialization order fiasco. T contains two static data members, smInstance and smMutex. The pointer could be statically created without any worries about other static dependencies and I'm assuming the same for smMutex. You could create the singleton as soon as you enter the main function and then spawn off those threads that will eventually grab the singleton instance pointer later.
In general, I was wondering, do you need to synchronize access to a block of memory that is being read simultaneously by many threads to ensure thread safety?
In this case, smInstance is that shared block of memory.
X *getInstance(){ if (smInstance == NULL) { 1) X *x = new X(); // if smInstance is null, set it to x if ( !CAS( smInstance, null, x ) ) { 2) // smInstance wasn't null, // someone modified in between 1) and 2), // so delete local copy delete x; } } return smInstance;}volatile X *smInstance;Is the above code thread safe, even though smInstance can be read at the same time by many threads?
Why is smInstance volatile?
Thanks, I appreciate the responses.
Generally its safe for multiple threads to be reading the same memory as long as no threads are writing.
When you declare a variable as volatile it lets the compiler know that the variable may be altered by outside forces, so when it does computations on the variable it won't cache the value, etc.
When you declare a variable as volatile it lets the compiler know that the variable may be altered by outside forces, so when it does computations on the variable it won't cache the value, etc.
Quote:In general, I was wondering, do you need to synchronize access to a block of memory that is being read simultaneously by many threads to ensure thread safety?
If the value of the variable can change, then yes. Static initialization means the variable can change.
While it can be avoided in certain cases, it has a subtle flaw. Unless the variable is marked as volatile, on multicore systems the variable may be accessed and cached for different periods of time on different cores. This can happen:
0) x has unknown value1) create threads2) x = 17;3) start threads4) each thread reads x and displays itThis may look like safe code. After all, x was written in non-threaded environment. Not on multi-core. You have no guarantee that all cores have flushed the old value of x, even though it was set in non-threaded mode.
This is an incredibly rare, yet so subtle flaw that can make debugging nightmare.
Generally, unless you have hardware guarantee, the write itself is *not* required to be atomic. So you could end up with value that has only written first 16 bits of a 32-bit variable.
There are two ways to thread-safety: Thread-safe, and non Thread-safe. There is no half-safe way.
Quote:I read about that static initialization order fiasco. T contains two static data members, smInstance and smMutex.
They don't need to be static. They just need to be global, defined in different units.
Quote:Is the above code thread safe, even though smInstance can be read at the same time by many threads?
It's safe, if smInstance is initialized to NULL before first access. C++ doesn't do that by itself.
Quote:Why is smInstance volatile?
See above, for multi-core issues with variable caching.
Quote:Why is T no longer a singleton if you can control the life-cycle?
Because then it becomes wrapper to static variable. Singleton is defined by the very fact that it creates itself when first called.
Quote:You still need to guarantee that only one instance of T can be created.
This has nothing to do with singletons. Singletons ensure that you always get the same instance, not that you cannot have more than one (this case doesn't happen as a consequence of previous rule, but it doesn't prevent you from spawning multiple instances of underlying object). Not making getInstance() thread-safe you no longer have that guarantee.
Quote:You could do this but it is no longer a singleton. How can you enforce that only one instance of T is created?
The same way you want to initialize the singleton manually to make sure it's valid (by coding carefully). You can prevent multiple instances by declaring T as non-copyable (private assignment operator).
Singletons are really really annoying to do correctly, yet very easy to get working in 95% of the cases (or some 50% of the cases in multi-core systems).
Author
Quote:While it can be avoided in certain cases, it has a subtle flaw. Unless the variable is marked as volatile, on multicore systems the variable may be accessed and cached for different periods of time on different cores. This can happen:
0) x has unknown value
1) create threads
2) x = 17;
3) start threads
4) each thread reads x and displays it
This may look like safe code. After all, x was written in non-threaded environment. Not on multi-core. You have no guarantee that all cores have flushed the old value of x, even though it was set in non-threaded mode.
This is an incredibly rare, yet so subtle flaw that can make debugging nightmare.
So, the volatile keyword ensures that x gets read from main memory all the time.
This would remove any issues with the compiler caching the value in a register.
However even if x is qualified with volatile, could out-of-order execution cause each thread to display different values(initial garbage value or 17)?
Quote:Quote:Why is T no longer a singleton if you can control the life-cycle?
Because then it becomes wrapper to static variable. Singleton is defined by the very fact that it creates itself when first called.
This makes sense now. I guess you really don't need that comparison for 0 if you know it will not be 0.
Quote:Quote:You still need to guarantee that only one instance of T can be created.
This has nothing to do with singletons. Singletons ensure that you always get the same instance, not that you cannot have more than one (this case doesn't happen as a consequence of previous rule, but it doesn't prevent you from spawning multiple instances of underlying object). Not making getInstance() thread-safe you no longer have that guarantee.
My design patterns book says the intent of the singleton is to ensure that a class only has one instance, and provide a global point of access to it.
This makes sense if the object is very expensive to create.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
