41

I write a singleton c++ in the follow way:

class A {
    private:
        static A* m_pA;
        A();
        virtual ~A();

    public:
        static A* GetInstance();
        static void FreeInstance();

        void WORK1();
        void WORK2();
        void WORK3();
    }
}

A* A::GetInstance() {
    if (m_pA == NULL)
        m_pA = new A();
    return m_pA;
}

A::~A() {
    FreeInstance()  // Can I write this? are there any potential error?
}

void A::FreeInstance() {
    delete m_pA;
    m_pA = NULL;
}

Thanks! Evan Teran and sep61.myopenid.com 's answer is right, and really good! My way is wrong, I wish any one writting such code can avoid my silly mistake.

My singleton A in my project has a vector of smart pointer, and another thread can also edit this vector, so when the application is closing, it always become unstable even I add lots of CMutex. Multithread error + singleton error wasted me 1 day.

//----------------------------------------------------------- A new singleton, you are welcome to edit if you think there is any problem in the following sample:

class A {
    private:
        static A* m_pA;
        explicit A();
        void A(const A& a);
        void A(A &a);
        const A& operator=(const A& a);
        virtual ~A();

    public:
        static A* GetInstance();
        static void FreeInstance();

        void WORK1();
        void WORK2();
        void WORK3();
    }
}

A* A::GetInstance() {
    if (m_pA == NULL){
        static A self;
        m_pA = &self;
    }
    return m_pA;
}

A::~A() {
}
FruitBreak
  • 570
  • 1
  • 7
  • 19
user25749
  • 4,825
  • 14
  • 61
  • 83
  • 5
    An interesting discussion on how to properly implement a singleton, along with thread-safety in C++ can be found in this paper: http://www.aristeia.com/Papers/DDJ%5FJul%5FAug%5F2004%5Frevised.pdf –  Oct 30 '09 at 11:36
  • Possible duplicate of [C++ Singleton design pattern](https://stackoverflow.com/questions/1008019/c-singleton-design-pattern). The https://stackoverflow.com/q/1008019/52074 is better because it 10x more upvotes and the question is more up-to-date with C++11 AND the **question/answer is being protected/maintained by the community**. – Trevor Boyd Smith Sep 04 '18 at 11:55

10 Answers10

210

Why does everybody want to return a singleton as a pointer?
Return it as a reference seems much more logical!

You should never be able to free a singleton manually. How do you know who is keeping a reference to the singleton? If you don't know (or can't guarantee) nobody has a reference (in your case via a pointer) then you have no business freeing the object.

Use the static in a function method.
This guarantees that it is created and destroyed only once. It also gives you lazy initialization for free.

class S
{
    public:
        static S& getInstance()
        {
            static S    instance;
            return instance;
        }
    private:
        S() {}
        S(S const&);              // Don't Implement.
        void operator=(S const&); // Don't implement
 };

Note you also need to make the constructor private. Also make sure that you override the default copy constructor and assignment operator so that you can not make a copy of the singleton (otherwise it would not be a singleton).

Also read:

To make sure you are using a singleton for the correct reasons.

Though technically not thread safe in the general case see:
What is the lifetime of a static variable in a C++ function?

GCC has an explicit patch to compensate for this:
http://gcc.gnu.org/ml/gcc-patches/2004-09/msg00265.html

Community
  • 1
  • 1
Martin York
  • 257,169
  • 86
  • 333
  • 562
  • 7
    Another benefit of references is that it is not possible to do this: 'delete A::getInstance()', which is possible with pointers.; – yoav.aviram Nov 07 '08 at 10:56
  • 2
    I guess everybody wants to optimize prematurely, and they feel like in control when they do the pointer thing. However, even the 'static S instance' clause is to be compiled to a thread-safe construct! Is it? – xtofl Nov 07 '08 at 11:17
  • 1
    Shouldn't the declaration of the assignment operator be `S& operator=(S const&);`? – kol Oct 12 '14 at 16:15
  • That is just one way of doing it. There is no standard way. Please try writing the code and verify to yourself that you can not use the assignment operator without a compile time error. The way you write it implies that assignment chaining can be done. Which makes no sense as this is a singleton so there should be no objects to chain together. So in my mind logically this operator= returns void. But in your singelton you may write it to return a reference and it will make no difference. – Martin York Oct 12 '14 at 16:20
  • 1
    I know why the assignment operator of a singleton is private, and why it's not implemented. But I think the code is bit more readable if one uses the more common version of the assignment operator, the one which returns a reference to the assignee: http://en.wikipedia.org/wiki/Assignment_operator_(C%2B%2B)#Return_value_of_overloaded_assignment_operator – kol Oct 12 '14 at 16:40
  • @kol. The reason I do it this way is because I don't blindly copy and paste code. I think about the usage semantics of the object (which is what you should do for all types). The default generated version is defined as it is because it has to fit certain backward compatibility constraints from C. You should learn to use the appropriate syntax for the class you are defining. In this situation it is not appropriate and more information is conveyed about that by being explicit. – Martin York Oct 12 '14 at 17:01
  • @kol: Note: in the link you provide they explicitly say that the return type of reference to the current type is used to allow chaining. How is that appropriate in any way to this situation. – Martin York Oct 12 '14 at 17:04
  • @kol: If you have explicit feelings on the subject start a question about return type of assignment operator and singeltons as a new question. Get group consensus (in another question). Please leave this one as is. – Martin York Oct 12 '14 at 17:05
  • 1
    @kol has a completely reasonable point. Sure, you can't chain a singleton. Well, you can, if you chain assignments to the same object. Regardless, I would not expect to see a strange API just because of that. It would be rather like defining your own custom integer type for every set of `int`s that you want with a different logical range. Y'know, because my "hours" counter can't go above 23, why would I want it to be able to go up to 2 billion? I'm glad that you "don't blindly copy and paste code" but on the flip side that is not a good reason alone to stubbornly stick to surprising attitudes. – Lightness Races in Orbit Apr 02 '15 at 23:06
  • @LightningRacisinObrit: Are you saying that my stance on not implementing an assignment operator is surprising? But I am open to reviewing alternatives given more space (which is not in comments it would need a whole new question as I asked kol to do). – Martin York Apr 11 '15 at 02:57
  • 1
    where do you write the destructor, public or private ? – mr_T Feb 02 '16 at 10:44
  • But the destructor would be called only when the program terminate, it's a waste of memory. – choxsword Jun 05 '18 at 14:53
  • 1
    @bigxiao Correct. But this is only a template example. In a real life this is a situation were you want to guarantee that the destructor is run. Like your communications link to the ISS (you want it to shut down correctly and not waste resources). PS. There is no waste of waste in memory as it uses the same amount of memory as any other technique. PS. Also note singeltons are usually a mistake. But this is how you do it iff you want one. – Martin York Jun 05 '18 at 14:57
  • @MartinYork Do you mean that there are no techniques to implement a singleton and release its resource before program terminating, even with a smart pointer? – choxsword Jun 05 '18 at 15:01
  • @bigxiao I never said that. – Martin York Jun 05 '18 at 15:03
  • Nice solution, to avoid a run-time memory allocation. But a silly question: How do you use it? I tried "main() { S instance = S.getInstance(); }". Not accepted by the compiler, "error: expected primary-expression before '.' token" – Paul Nov 26 '21 at 15:16
  • Found it: Use S::getInstance() – Paul Nov 26 '21 at 15:25
  • 1
    @Paul I know my comment is late, but you could also declare your variable to have type S&. – Tunococ Jul 14 '23 at 09:03
13

You can avoid needing to delete it by using a static object like this:

if(m_pA == 0) {
    static A static_instance;
    m_pA = &static_instance;
}
Evan Teran
  • 87,561
  • 32
  • 179
  • 238
  • 4
    Big note: if you are using multithreading, you should be incredibly careful about doing this. See http://stackoverflow.com/questions/246564/what-is-the-lifetime-of-a-static-variable-in-a-c-function for why it is a very very bad idea in general. – hazzen Nov 07 '08 at 02:57
  • very good point, which is why when using multiple threads there should be mutexes or some other synchronization primitives involved. – Evan Teran Nov 07 '08 at 03:00
  • 2
    Just don't even try and implement it using double locking, because that can NOT be made to work correctly in C++ (see google for details). – Martin York Nov 07 '08 at 03:04
  • Add gcc has an explicit patch to (read non-standard) to cope with this. – Martin York Nov 07 '08 at 03:06
  • 2
    The best way is to decouple referencing and creation and have the main thread create all singletons before starting any threads that may reference a singleton. Btw returning a reference is in most cases superior to returning a pointer... – Andreas Magnusson Nov 07 '08 at 09:16
  • 4
    http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf is a good reference on the double checked locking issues in C++. – xtofl Nov 07 '08 at 11:20
  • Gotta love the random downvote 2 years after the fact with no explanation :-P – Evan Teran Oct 07 '10 at 17:00
4

A singleton in C++ can be written in this way:

static A* A::GetInstance() {
    static A sin;
    return &sin;
}
sep
  • 3,409
  • 4
  • 29
  • 32
3

Just don't forget to make the copy constructor and assignment operators private.

Jasper Bekkers
  • 6,711
  • 32
  • 46
1

I do not think there is any reason to write that line no. Your destructor method is not static and your singleton instance will not be destructed in that fashion. I do not think the destructor is necessary, if you need to cleanup the object use the static method you've alread created, FreeInstance().

Other than that, you create your singletons in roughly the same way that I create mine.

Odd
  • 4,737
  • 6
  • 30
  • 27
1

After a period of wild enthusiasm for Meyers-style singletons (using local static objects as in some of the previous answers), I got completely sick of the lifetime management problems in complicated apps.

I tend to find that you end up referencing the 'Instance' method deliberately early in the app's initialisation, to make sure they're created when you want, and then playing all kinds of games with the tear-down because of the unpredictable (or at least very complicated and somewhat hidden) order in which things get destroyed.

YMMV of course, and it depends a bit on the nature of the singleton itself, but a lot of the waffle about clever singletons (and the threading/locking issues which surround the cleverness) is overrated IMO.

Will Dean
  • 39,055
  • 11
  • 90
  • 118
1

if you read "Modern C++ Design" you'll realize that a singleton design could be much complex than return a static variable.

jab
  • 2,844
  • 1
  • 21
  • 22
0

There is a great C++ library, ACE, based on patterns. There's a lot of documentation about different kind of patterns so look at their work: http://www.cs.wustl.edu/~schmidt/ACE.html

mjk
  • 2,443
  • 4
  • 33
  • 33
0

This implementation is fine as long as you can answer these questions:

  1. do you know when the object will be created (if you use a static object instead of new? Do you have a main()?)

  2. does you singleton have any dependencies that may not be ready by the time it is created? If you use a static object instead of new, what libraries have been initialized by this time? What your object does in constructor that might require them?

  3. when will it be deleted?

Using new() is safer because you control where and when the object will be created and deleted. But then you need to delete it explicitly and probably nobody in the system knows when to do so. You may use atexit() for that, if it makes sense.

Using a static object in method means that do do not really know when it will be created or deleted. You could as well use a global static object in a namespace and avoid getInstance() at all - it doesn't add much.

If you do use threads, then you're in big trouble. It is virtually impossible to create usable thread safe singleton in C++ due to:

  1. permanent lock in getInstance is very heavy - a full context switch at every getInstance()
  2. double checked lock fails due to compiler optimizations and cache/weak memory model, is very tricky to implement, and impossible to test. I wouldn't attempt to do it in a real system, unless you intimately know your architecture and want it to be not portable

These can be Googled easily, but here's a good link on weak memory model: http://ridiculousfish.com/blog/archives/2007/02/17/barrier.

One solution would be to use locking but require that users cache the pointer they get from getInctance() and be prepared for getInstance() to be heavy.

Another solution would be to let users handle thread safety themselves.

Yet another solution would be to use a function with simple locking and substitute it with another function without locking and checking once the new() has been called. This works, but full implementation is complicated.

n-alexander
  • 14,663
  • 12
  • 42
  • 43
0
//! @file singleton.h
//!
//! @brief Variadic template to make a singleton out of an ordinary type.
//!
//! This template makes a singleton out of a type without a default
//! constructor.

#ifndef SINGLETON_H
#define SINGLETON_H

#include <stdexcept>

template <typename C, typename ...Args>
class singleton
{
private:
  singleton() = default;
  static C* m_instance;

public:
  singleton(const singleton&) = delete;
  singleton& operator=(const singleton&) = delete;
  singleton(singleton&&) = delete;
  singleton& operator=(singleton&&) = delete;

  ~singleton()
  {
    delete m_instance;
    m_instance = nullptr;
  }

  static C& create(Args...args)
  {
    if (m_instance != nullptr)
      {
    delete m_instance;
    m_instance = nullptr;
      }
    m_instance = new C(args...);
    return *m_instance;
  }

  static C& instance()
  {
    if (m_instance == nullptr)
      throw std::logic_error(
        "singleton<>::create(...) must precede singleton<>::instance()");
    return *m_instance;
  }
};

template <typename C, typename ...Args>
C* singleton<C, Args...>::m_instance = nullptr;

#endif // SINGLETON_H