views:

500

answers:

3

I would like to create a singleton class that is instantiated once in each thread where it is used. I would like to store the instance pointers in TLS slots. I have come up with the following solution but I am not sure whether there are any special considerations with multithreaded access to the singelton factory when thread local storage is involved. Maybe there is also a better solution to implement thread local singletons.

class ThreadLocalSingleton 
{
    static DWORD tlsIndex;
public:
    static ThreadLocalSingleton *getInstance()
    {
        ThreadLocalSingleton *instance = 
            static_cast<ThreadLocalSingleton*>(TlsGetValue(tlsIndex));
        if (!instance) {
            instance = new ThreadLocalSingleton();
            TlsSetValue(tlsIndex, instance);
        }
        return instance;
    }
};
DWORD ThreadLocalSingleton::tlsIndex = TlsAlloc();

The Tls*-functions are of course win32 specific but portability is not the main issue here. Your thoughts concerning other platforms would still be valuable.

Major Edit: I had originally asked about using double-checked locking in this scenario. However as DavidK pointed out, the singletons are to be created on a per thread basis anyway.

The two remaining questions are:

  1. is it appropriate to reply on TlsGetValue/TlsSetValue to ensure that each thread gets one instance and that the instance is created only once for each thread?

  2. Is it possible to register a callback that allows me to clean up an instance that was associated with a particular thread when that thread finishes?

+4  A: 

Have a look at this paper to understand why double-checked locking doesn't work in general (even though it might work in special cases).

sbi
The volatile keyword is supposed to fix this issue in the microsoft compiler since VS2005. See http://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Microsoft_Visual_C.2B.2B
VoidPointer
However, regarding the use of `volatile` for this, VC _is_ a special case. In general, this won't work.
sbi
Correct... a link to this paper is always in order when DCLP is discussed :)
VoidPointer
+9  A: 

Since your objects are thread-local, why do you need locking to protect them at all? Each threads that calls getInstance() will be independent of any other thread, so why not just check that the singleton exists and create it if needed? The locking would only be needed if multiple threads tried to access the same singleton, which isn't possible in your design as it is above.

EDIT: Moving on to the two other questions... I can't see any reason why using TlsAlloc/TlsGetValue etc. wouldn't work as you'd expect. Since the memory holding the pointer to your singleton is only accessible to the relevant thread, there won't be any problems with a lazy initialization of it. However there's no explicit callback interface to clean them up.

The obvious solution to that would be to have a method that is called by all your thread main functions that clears up the created singleton, if any.

If it's very likely that the thread will create a singelton, a simpler pattern might be to create the singleton at the start of the thread main function and delete it at the end. You could then use RAII by either creating the singleton on the stack, or holding it in a std::auto_ptr<>, so that it gets deleted when the thread ends. (Unless the thread terminates abnormally, but if that happens all bets are off and a leaked object is the least of your problems.) You could then just pass the singleton around, or store it in TLS, or store it in a member of a class, if most of the thread functionality is in one class.

DavidK
Well thanks. I was obviously in way-too-complicated mode.
VoidPointer
+1  A: 

We use a class that stores a map of thread id to data to implement our thread local storage. This seems to work very well, then an instance of this class can be placed anywhere you require thread local storage. Normally clients use an instance of as a static private field.

Here is a rough outline of the code

template <class T>
struct ThreadLocal {
    T & value()
    {
     LockGuard<CriticalSection> lock(m_cs);

     std::map<int, T>::iterator itr = m_threadMap.find(Thread::getThreadID());

     if(itr != m_threadMap.end())
      return itr->second;

     return m_threadMap.insert(
      std::map<int, T>::value_type(BWThread::getThreadID(), T()))
       .first->second;
    }

    CriticalSection  m_cs;
    std::map<int, T> m_threadMap;
};

This is then used as

class A {
    // ...

    void doStuff();
private:
   static ThreadLocal<Foo> threadLocalFoo;
};

ThreadLocal<Foo> A::threadLocalFoo;

void A::doStuff() {
    // ...
    threadLocalFoo.value().bar();
    // ...
}

This is simple and works on any platform where you can get the thread id. Note the Critical Section is only used to return/create the reference, once you have the reference all calls are outside the critical section.

iain