Singleton with private destructor using std::unique_ptr

Question

I've created all singletons in my program with that document in mind: http://erdani.com/publications/DDJ_Jul_Aug_2004_revised.pdf (in case anyone wondered why singleton, all of them are factories and some of them store some global settings concerning how they should create instances).

Each of them looks somehow like this:

declaration:

class SingletonAndFactory {
    static SingletonAndFactory* volatile instance;

public:
    static SingletonAndFactory& getInstance();

private:
    SingletonAndFactory();

    SingletonAndFactory(
        const SingletonAndFactory& ingletonFactory
    );

    ~SingletonAndFactory();
};

definition:

boost::mutex singletonAndFactoryMutex;

////////////////////////////////////////////////////////////////////////////////

// class SingletonAndFactory {

SingletonAndFactory* volatile singletonAndFactory::instance = 0;

// public:

SingletonAndFactory& SingletonAndFactory::getInstance() {
    // Singleton implemented according to:
    // "C++ and the Perils of Double-Checked Locking".
    if (!instance) {
        boost::mutex::scoped_lock lock(SingletonAndFactoryMutex);
        if (!instance) {
            SingletonAndFactory* volatile tmp = (SingletonAndFactory*) malloc(sizeof(SingletonAndFactory));
            new (tmp) SingletonAndFactory; // placement new
            instance = tmp;
        }
    }
    return *instance;
}

// private:

SingletonAndFactory::SingletonAndFactory() {}

// };

Putting aside question what design of singleton is the best (since it would start a pointless flame war) my question is: would it benefit me to replace normal pointer with std::unique_ptr? In particular, would it call singleton's destructor on program exit? If so how would I achieve it? When I tried to add something like friend class std::unique_ptr<SingletonAndFactory>; it didn't worked out since compiler keep on complaining that the destructor is private.

I know it doesn't matter in my current project since none of factories have something that would require cleaning of any sort, but for future reference I would like to know how to implement such behavior.

Answer 1

It's not the unique_ptr itself that does the deletion, it's the deleter. So if you wanted to go with the friend approach, you'd have to do this:

friend std::unique_ptr<SingletonFactory>::deleter_type;

However, I don't think it's guaranteed that the default deleter will not delegate the actual delete to another function, which would break this.

Instead, you might want to supply your own deleter, perhaps like this:

class SingletonFactory {
    static std::unique_ptr<SingletonFactory, void (*)(SingletonFactory*)> volatile instance;

public:
    static SingletonFactory& getInstance();

private:
    SingletonFactory();

    SingletonFactory(
        const SingletonFactory& ingletonFactory
    );

    ~SingletonFactory();

    void deleter(SingletonFactory *d) { d->~SingletonFactory(); free(d); }
};

And in the creation function:

SingletonFactory* volatile tmp = (SingletonFactory*) malloc(sizeof(SingletonFactory));
new (tmp) SingletonFactory; // placement new
instance = decltype(instance)(tmp, &deleter);

Answer 2

In C++11, you can guarantee thread-safe lazy initialisation and destruction at the end of the program using a local static:

SingletonAndFactory& SingletonAndFactory::getInstance() {
    static SingletonAndFactory instance;
    return instance;
}

Beware that this can still cause lifetime issues, as it may be destroyed before other static objects. If they try to access it from their destructors, then you'll be in trouble.

Before that, it was impossible (although the above was guaranteed by many compilers). As described in the document you link to, volatile has nothing to do with thread synchronisation, so your code has a data race and undefined behaviour. Options are:

• Take the (potentially large) performance hit of locking to check the pointer
• Use whatever non-portable atomic intrinsics your compiler provides to test the pointer
• Forget about thread-safe initialisation, and make sure it's initialised before you start your threads
• Don't use singletons

I favour the last option, since it solves all the other problems introduced by the Singleton anti-pattern.