-2

This is a follow up of this post on double checked locking. I am writing a new post because it seems that posting a follow-up on "aged" posts doesn't make it as visible/active as sending out a new post, maybe because most people do not sort posts in stackoverflow by level of activity.

To all who responded, thank you for all your input in the original post. After consulting Joe Duffy's excellent book, "Concurrent Programming on Windows", I am now thinking that I should be using the code below. It's largely identical to the code from his book, except for some variable renamings and the InterlockedXXX line. The following implementation uses:

  1. volatile keyword on both the temp and "actual" pointers to protect against re-ordering from the compiler.
  2. InterlockedCompareExchangePointer to protect against reordering from the CPU.

So, that should be pretty safe (... right?):

template <typename T>
class LazyInit {
public:
    typedef T* (*Factory)();
    LazyInit(Factory f = 0) 
        : factory_(f)
        , singleton_(0)
    {
        ::InitializeCriticalSection(&cs_);
    }

    T& get() {
        if (!singleton_) {
            ::EnterCriticalSection(&cs_);
            if (!singleton_) {
                T* volatile p = factory_();
                // Joe uses _WriterBarrier(); then singleton_ = p;
                // But I thought better to make singleton_ = p atomic (as I understand, 
                // on Windows, pointer assignments are atomic ONLY if they are aligned)
                // In addition, the MSDN docs say that InterlockedCompareExchangePointer
                // sets up a full memory barrier.
                ::InterlockedCompareExchangePointer((PVOID volatile*)&singleton_, p, 0);
            }
            ::LeaveCriticalSection(&cs_);
        }
        #if PREVENT_IA64_FROM_REORDERING
        _ReadBarrier();
        #endif
        return *singleton_;
    }

    virtual ~LazyInit() {
        ::DeleteCriticalSection(&cs_);
    }
private:
    CRITICAL_SECTION cs_;
    Factory factory_;
    T* volatile singleton_;
};
Community
  • 1
  • 1
moog
  • 383
  • 5
  • 13
  • 2
    Just so you know, you didn't post a follow-up, you posted an answer to your own question. This isn't a discussion forum, it's a collection of questions with answers. Answers generally aren't ordered chronologically, so they don't form a thread. The only place conversations can take place is in comments. – Steve Jessop Aug 21 '10 at 12:26
  • 1
    And you're just confusing people when you post new questions continuing on from, and referring to, earlier questions. Assume that we haven't read your last question, or if we have, we can't remember it. Try to make each of your questions stand alone – jalf Aug 21 '10 at 13:36
  • 1
    And on another note, doesn't it strike you as a waste of effort, adding so much complexity just to get a singleton that *maybe* works, when you could have just avoided the singleton in the first place and been better off for it? http://jalf.dk/blog/2010/03/singletons-solving-problems-you-didnt-know-you-never-had-since-1995/ – jalf Aug 21 '10 at 13:36
  • 1
    If we are talking about Singeltons is completely useless as there is no guaranteed destruction (as shown in this context). – Martin York Aug 21 '10 at 14:30
  • Who up-voted this? I voted for closing as not a real question. – sbi Aug 21 '10 at 19:08

1 Answers1

2

I've always used a much simpler singleton pattern.

class CSomething
{
protected:
    CSomething() {};
public:
    ~CSomething() {};
    static CSomething *Get()
    {
        static CSomething s;
        return &s;
    }
    // Rest of the class
};
Michael J
  • 7,631
  • 2
  • 24
  • 30