c++11 Singleton is creating new instances

Question

This is my singleton

class MySingleton {
private:
    int myNumber;
public:
    static MySingleton &get() {
        static MySingleton instance;
        return instance;
    }

    int getMyNumber() const {
        return myNumber;
    }

    void setMyNumber(int myNumber) {
        MySingleton::myNumber = myNumber;
    }
};

and this is the code to test it

MySingleton t1 = MySingleton::get();
t1.setMyNumber(6);
printf("t1: %d \n", t1.getMyNumber());

MySingleton t2 = MySingleton::get();
printf("t2: %d \n", t2.getMyNumber());

Now the output I get is

t1: 6 
t2: 0

but te expected result would be

t1: 6 
t2: 6

It looks like MySingleton::get() is creating a new instance, something it shouldn't do of course.

Why shouldn't it? You didn't forbid copying or assignment. The compiler has no way to decipher you want a singleton concept, you have to specify that behavior. — StoryTeller - Unslander Monica, Jul 24 '17 at 11:43
Do note that your singleton is not a singleton. As is it can be default constructed and copied. — NathanOliver, Jul 24 '17 at 11:44
when you call `setMyNumber(6)` you call it on a copy (named `t1`) of the singleton. The next call to `get` returns a reference to the same instance as before, just that it has no idea about the changes you made to the copy — 463035818_is_not_an_ai, Jul 24 '17 at 11:47

score 6 · Accepted Answer · answered Jul 24 '17 at 11:46

In your case get() isn't creating the copy. The assignment to t1 and t2 is doing it. To avoid it change those to references: MySingleton& t1 = MySingleton::get().

And in order to avoid accidental errors remove the copy and assignment operators:

MySingleton(const MySingleton& other) = delete;
MySingleton& operator=(const MySingleton& other) = delete;

Edgar Rokjān · Answer 2 · 2017-07-24T11:55:13.347

2

The reason is that you create a copy of the instance in these lines:

MySingleton t1 = MySingleton::get();
MySingleton t2 = MySingleton::get();

and change the copy.

To make it work you have to add a reference:

MySingleton& t1 = MySingleton::get();
MySingleton& t2 = MySingleton::get();

Also note, that the classic singleton design assumes deleted copy constructor and copy assignment operator to avoid such bugs:

class MySingleton {
    private:
        // ...
    public:
        // ...
        MySingleton(const MySingleton&) = delete;
        void operator=(const MySingleton&) = delete;
};

P.S. You might find this discussion useful. It shows the implementation details of a singleton pattern with C++.

edited Jul 24 '17 at 11:55

answered Jul 24 '17 at 11:48

Edgar Rokjān

17,245
4
40
67

Therefore needs to define a (`private`) constructor. `MySingleton():myNumber(0){};` maybe? – Persixty Jul 24 '17 at 12:37
1

@Persixty I've provided OP with a part of implementation which was relative to his problem. He might easily found the full implementation, so I've decided not to post it. And yes, constructor should be private, of course. – Edgar Rokjān Jul 24 '17 at 12:39

The Techel · Answer 3 · 2017-07-24T11:51:03.213

You are copying the singleton instead of obtaining a reference:

MySingleton &t1 = MySingleton::get();

Mark the copy- an move-assignment and -constructors as delete and make the constructor and destructor private:

class MySingleton
{
public:
    MySingleton(const MySingleton&) = delete;
    void operator=(const MySingleton&) = delete;

    MySingleton(MySingleton&&) = delete;
    void operator=(MySingleton&&) = delete;

private:
    MySingleton() = default;
    ~MySingleton() = default;

};

Note that you probably just want a global variable instead of a singleton.

score 0 · Answer 4 · answered Jul 24 '17 at 11:52

You declare both t1 and t2 as instances of the class rather than references to the class.

The get function is implemented correctly but then each instance is constructed upon assignment. Note that making your constructor private would have prevented this bug at compilation.

I suggest going through a tutorial on how to implement Singletons in c++ to learn some good practices, or simply using an existing well-made impelmentation.

c++11 Singleton is creating new instances

4 Answers4