Singleton object using unique_ptr

Question

I have the below CPP source code for creating a Singleton Object of a class using unique_ptr:

#include <iostream>
#include <memory>

class A
{

public:
    std::unique_ptr<A> getInstance(int log);
    ~A();

private:
    static bool instanceFlag;
    static std::unique_ptr<A> single;
    A(int log);
    int mLog;
};

bool A::instanceFlag = false;
std::unique_ptr<A> A::single = NULL;

std::unique_ptr<A> A::getInstance(int log)
{
    if(!instanceFlag)
    {
        //single = std::make_unique<A>(log);
        single = std::unique_ptr<A>(new A(log));
        instanceFlag = true;
        return std::move(single);
    }
    else
    {
        return std::move(single);
    }
}

A::A(int log) :
    mLog(log)
{

    std::cout << "Called A cons" << std::flush << std::endl;
}

int main()
{
std::unique_ptr<A> mA = A::getInstance(5);
}

But when I compile the code I get below error:

$ c++ -std=c++11 try2.cpp
try2.cpp: In function 'int main()':
try2.cpp:45:41: error: cannot call member function 'std::unique_ptr<A> A::getInstance(int)' without object
 std::unique_ptr<A> mA = A::getInstance(5);
                                         ^

However I have exactly the same format of code in my project I get an error :

Source code Line 39: single = std::make_unique<A>(log);

Compilation error:

39:   required from here

single = std::make_unique<A>(log);

error: A(int log)' is private
 A::A(int log) :
  ^

Answer 1

First, the static issue. This:

std::unique_ptr<A> getInstance(int log);

is an instance method. You need an instance of A to call it on, but you can't get an instance of A without calling this method first, so ...

The reason for using instance methods is that they have access to the instance they're invoked on. Your method only uses the following members:

static bool instanceFlag;
static std::unique_ptr<A> single;

which, since they're static, don't need an instance. Just make the method static too, and you'll be able to call it without first getting an A from somewhere.

---

Second, the logic issue. Your getInstance method returns a unique_ptr to your single instance. The entire point of unique_ptr is that's it's unique: exactly one pointer owns and controls the lifetime of an object. When you returned a unique_ptr, you transferred ownership of the singleton object from the singleton class itself, to the caller. You even explicitly called move to make it really clear that this is happening.

Now, after calling getInstance once, your singleton is completely broken. If you call getInstance again, the singleton believes it has an instance (because instanceFlag), but the unique_ptr is in an indeterminate state. The only thing we can say for sure is that it doesn't control an instance of A.

Just return a raw pointer (or reference) to A instead of transferring ownership.

Answer 2

Even after making the getInstance static, it won't compile because the definition for destructor is missing. Either give empty definition or leave it as default.

~A() = default;

Moving single will work first time and invokes UB for subsequent calls to getInstance. Instead return by reference & and remove std::move

static std::unique_ptr<A>& getInstance(int log);

std::unique_ptr<A>& A::getInstance(int log)
{
    if(!instanceFlag)
    {
        //single = std::make_unique<A>(log);
        single = std::unique_ptr<A>(new A(log));
        instanceFlag = true;
        return single;
    }
    else
    {
        return single;
    }
}

Inside main() you can get by reference std::unique_ptr<A>& mA = A::getInstance(5);