Singleton being constructed twice

Question

I have the following code:

#include <iostream>
#include <fstream>
#include <string>
#include <cassert>
#include <memory>

class Logger : public std::ofstream
{
public:
    explicit Logger(std::string const& filename = "log.txt"):
        std::ofstream(filename.c_str())
    {
        assert(*this);

        *this << "-- log file start --\n" << std::endl;
    }

    Logger::~Logger()
    {
        *this << "\n-- log file end --" << std::endl;

        this->close();
        this->clear();
    }
};

Logger logger;

template<class T>
class NotCopyable
{
public:

    NotCopyable() { }
    ~NotCopyable() { }

private:

    NotCopyable(NotCopyable const&);
    NotCopyable const& operator=(NotCopyable const&);
};

template<class T>
class Singleton : public NotCopyable<Singleton<T> >
{
public:

    static T& GetInstance()
    {
        if (!instance)
        {
            logger << "Initialize Singleton" << std::endl;

            instance.reset(new T());
        }

        return *instance;
    }

protected:

    Singleton() { }
    virtual ~Singleton() { }

private:

    static std::unique_ptr<T> instance;
};

template<class T>
std::unique_ptr<T> Singleton<T>::instance;

class Factory : public Singleton<Factory>
{
public:

    Factory() { logger << "Factory constructor" << std::endl; }
    ~Factory() { logger << "Factory destructor" << std::endl; }

    void Blargl() { logger << "Blargl" << std::endl; }

};

bool DoStuff()
{
    Factory::GetInstance().Blargl();

    return true;
}

bool Thingy = DoStuff();

int main(int, char*[])
{
    logger << "Start main()" << std::endl;

    Factory::GetInstance().Blargl();

    logger << "End main()" << std::endl;
}

This outputs the following:

-- log file start --

Initialize Singleton
Factory constructor
Blargl
Start main()
Initialize Singleton
Factory constructor
Blargl
End main()
Factory destructor

-- log file end --

I feel stupid, but can't see why the factory is being constructed twice instead of once. What's going on here?

Answer 1

I tried to run your code, and had the same behaviour you described on OSX, Apple llvm 5.0.
It works fine if you define the static instance variable inside the GetInstance() method:

static T& GetInstance()
{
    static std::unique_ptr<T> instance
    if (!instance)
    {
        logger << "Initialize Singleton" << std::endl;

        instance.reset(new T());
    }

    return *instance;
}

I think that the problem you have in your code is the unspecified order of execution between the initialization of the Singleton::instance at its declaration point (default constructor), and the assignment in the GetInstance() method.
So, if what I am saying is right, the order of execution is probably something like:

1. GetInstance() call from DoStuff() call
2. Default-construction of the Singleton<Factory>::instance
3. new GetInstance() call inside main()

---

Edit: tested my theory with the following code:

template <typename T>
class Ptr {
public:
    Ptr()
    : p() {
        std::cout << "Initalizing Ptr" << std::endl;
    }

    std::unique_ptr<T> p;
};



template<class T>
class Singleton : public NotCopyable<Singleton<T>> {
public:
    static T& GetInstance()
    {
        if (!instance.p)
        {
            std::cout << "Initalizing Singleton" << std::endl;
            logger << "Initialize Singleton" << std::endl;
            instance.p.reset(new T());
        }
    return *instance.p;
}

protected:
    Singleton() { }
    virtual ~Singleton() { }

private:
    static Ptr<T> instance;
};

template<class T>
Ptr<T> Singleton<T>::instance;

Output:

1. Initalizing Singleton
2. Initalizing Ptr
3. Initalizing Singleton